Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: alsa-devel@alsa-project.org
Cc: Curtis Malainey <cujomalainey@google.com>
Subject: [PATCH RFC 3/9] ALSA: pcm: Don't embed device
Date: Wed, 16 Aug 2023 18:02:46 +0200	[thread overview]
Message-ID: <20230816160252.23396-4-tiwai@suse.de> (raw)
In-Reply-To: <20230816160252.23396-1-tiwai@suse.de>

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
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h             |  2 +-
 sound/aoa/soundbus/i2sbus/pcm.c |  4 ++--
 sound/core/pcm.c                | 22 +++++++++++++---------
 sound/usb/media.c               |  2 +-
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 19f564606ac4..0243a13e9ac4 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -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 {
diff --git a/sound/aoa/soundbus/i2sbus/pcm.c b/sound/aoa/soundbus/i2sbus/pcm.c
index a9e502a6cdeb..3680eb6eabc9 100644
--- a/sound/aoa/soundbus/i2sbus/pcm.c
+++ b/sound/aoa/soundbus/i2sbus/pcm.c
@@ -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;
 	}
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 9d95e3731123..317a25b68159 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -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);
@@ -847,7 +850,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)
@@ -1017,7 +1020,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] = {
@@ -1078,7 +1081,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;
@@ -1125,7 +1128,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);
diff --git a/sound/usb/media.c b/sound/usb/media.c
index 6d11fedb4632..d48db6f3ae65 100644
--- a/sound/usb/media.c
+++ b/sound/usb/media.c
@@ -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;
-- 
2.35.3


  parent reply	other threads:[~2023-08-16 16:05 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-16 16:02 [PATCH RFC 0/9] ALSA: Don't embed struct devices Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 1/9] ALSA: core: Introduce snd_device_alloc() Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 2/9] ALSA: control: Don't embed ctl_dev Takashi Iwai
2023-08-16 16:02 ` Takashi Iwai [this message]
2023-08-16 16:02 ` [PATCH RFC 4/9] ALSA: hwdep: Don't embed device Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 5/9] ALSA: rawmidi: " Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 6/9] ALSA: compress: " Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 7/9] ALSA: timer: Create device with snd_device_alloc() Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 8/9] ALSA: seq: " Takashi Iwai
2023-08-16 16:02 ` [PATCH RFC 9/9] ALSA: core: Drop snd_device_initialize() Takashi Iwai
2023-08-16 16:45 ` [PATCH RFC 0/9] ALSA: Don't embed struct devices Jaroslav Kysela
2023-08-16 22:18   ` Curtis Malainey
2023-08-17  7:27 ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230816160252.23396-4-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=cujomalainey@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox