Commit bc41a722 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: pcm: Don't embed device 

So far we use the embedded struct device for each PCM substreams in
struct snd_pcm.  This may result in UAF when the delayed kobj release
is used; each corresponding struct device is still accessed at the
(delayed) device release, while the snd_pcm object may be already
gone.

As a workaround, detach the struct device from the snd_pcm object by
allocating via the new snd_device_alloc() helper.

A caveat is that we store the PCM substream pointer to drvdata since
the device resume and others require the access to it.

This patch is based on the fix Curtis posted initially.  In this
patch, the changes are split and use the new helper function instead.

Link: https://lore.kernel.org/r/20230801171928.1460120-1-cujomalainey@chromium.org


Reviewed-by: default avatarJaroslav Kysela <perex@perex.cz>
Signed-off-by: default avatarCurtis Malainey <cujomalainey@chromium.org>
Tested-by: default avatarCurtis Malainey <cujomalainey@chromium.org>
Link: https://lore.kernel.org/r/20230816160252.23396-4-tiwai@suse.de


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 6a66b01d

include/sound/pcm.h

+1 −1
Original line number Diff line number Diff line
@@ -510,7 +510,7 @@ struct snd_pcm_str { 
#endif

#endif

	struct snd_kcontrol *chmap_kctl; /* channel-mapping controls */

	struct device dev;

	struct device *dev;

};



struct snd_pcm {

sound/aoa/soundbus/i2sbus/pcm.c

+2 −2
Original line number Diff line number Diff line
@@ -972,7 +972,7 @@ i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card, 
			goto out_put_ci_module;

		snd_pcm_set_ops(dev->pcm, SNDRV_PCM_STREAM_PLAYBACK,

				&i2sbus_playback_ops);

		dev->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].dev.parent =

		dev->pcm->streams[SNDRV_PCM_STREAM_PLAYBACK]->dev.parent =

			&dev->ofdev.dev;

		i2sdev->out.created = 1;

	}
@@ -989,7 +989,7 @@ i2sbus_attach_codec(struct soundbus_dev *dev, struct snd_card *card, 
			goto out_put_ci_module;

		snd_pcm_set_ops(dev->pcm, SNDRV_PCM_STREAM_CAPTURE,

				&i2sbus_record_ops);

		dev->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].dev.parent =

		dev->pcm->streams[SNDRV_PCM_STREAM_CAPTURE]->dev.parent =

			&dev->ofdev.dev;

		i2sdev->in.created = 1;

	}

sound/core/pcm.c

+13 −9
Original line number Diff line number Diff line
@@ -604,7 +604,7 @@ static const struct attribute_group *pcm_dev_attr_groups[]; 
#ifdef CONFIG_PM_SLEEP

static int do_pcm_suspend(struct device *dev)

{

	struct snd_pcm_str *pstr = container_of(dev, struct snd_pcm_str, dev);

	struct snd_pcm_str *pstr = dev_get_drvdata(dev);



	if (!pstr->pcm->no_device_suspend)

		snd_pcm_suspend_all(pstr->pcm);
@@ -650,11 +650,14 @@ int snd_pcm_new_stream(struct snd_pcm *pcm, int stream, int substream_count) 
	if (!substream_count)

		return 0;



	snd_device_initialize(&pstr->dev, pcm->card);

	pstr->dev.groups = pcm_dev_attr_groups;

	pstr->dev.type = &pcm_dev_type;

	dev_set_name(&pstr->dev, "pcmC%iD%i%c", pcm->card->number, pcm->device,

	err = snd_device_alloc(&pstr->dev, pcm->card);

	if (err < 0)

		return err;

	dev_set_name(pstr->dev, "pcmC%iD%i%c", pcm->card->number, pcm->device,

		     stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c');

	pstr->dev->groups = pcm_dev_attr_groups;

	pstr->dev->type = &pcm_dev_type;

	dev_set_drvdata(pstr->dev, pstr);



	if (!pcm->internal) {

		err = snd_pcm_stream_proc_init(pstr);
@@ -845,7 +848,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr) 
#endif

	free_chmap(pstr);

	if (pstr->substream_count)

		put_device(&pstr->dev);

		put_device(pstr->dev);

}



#if IS_ENABLED(CONFIG_SND_PCM_OSS)
@@ -1015,7 +1018,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) 
static ssize_t pcm_class_show(struct device *dev,

			      struct device_attribute *attr, char *buf)

{

	struct snd_pcm_str *pstr = container_of(dev, struct snd_pcm_str, dev);

	struct snd_pcm_str *pstr = dev_get_drvdata(dev);

	struct snd_pcm *pcm = pstr->pcm;

	const char *str;

	static const char *strs[SNDRV_PCM_CLASS_LAST + 1] = {
@@ -1076,7 +1079,7 @@ static int snd_pcm_dev_register(struct snd_device *device) 
		/* register pcm */

		err = snd_register_device(devtype, pcm->card, pcm->device,

					  &snd_pcm_f_ops[cidx], pcm,

					  &pcm->streams[cidx].dev);

					  pcm->streams[cidx].dev);

		if (err < 0) {

			list_del_init(&pcm->list);

			goto unlock;
@@ -1123,7 +1126,8 @@ static int snd_pcm_dev_disconnect(struct snd_device *device) 


	pcm_call_notify(pcm, n_disconnect);

	for (cidx = 0; cidx < 2; cidx++) {

		snd_unregister_device(&pcm->streams[cidx].dev);

		if (pcm->streams[cidx].dev)

			snd_unregister_device(pcm->streams[cidx].dev);

		free_chmap(&pcm->streams[cidx]);

	}

	mutex_unlock(&pcm->open_mutex);

sound/usb/media.c

+1 −1
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ int snd_media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm, 
{

	struct media_device *mdev;

	struct media_ctl *mctl;

	struct device *pcm_dev = &pcm->streams[stream].dev;

	struct device *pcm_dev = pcm->streams[stream].dev;

	u32 intf_type;

	int ret = 0;

	u16 mixer_pad;