diff mbox series

[2/6] ALSA: core: Fix race between devres and delayed kobject release for card_dev

Message ID 20230807135207.17708-4-tiwai@suse.de (mailing list archive)
State New, archived
Headers show
Series ALSA: Fix UAF with delayed kobj release | expand

Commit Message

Takashi Iwai Aug. 7, 2023, 1:52 p.m. UTC 
Use a new refmem allocation for the card object, and fix the race
between the devres and the delayed kobj release.  Now the devres keeps
only the card object pointer, not the card object itself, and the card
object is unreferenced at both releases.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/init.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

Comments

Takashi Iwai Aug. 7, 2023, 1:56 p.m. UTC | #1 
On Mon, 07 Aug 2023 15:52:03 +0200,
Takashi Iwai wrote:
> 
> Use a new refmem allocation for the card object, and fix the race
> between the devres and the delayed kobj release.  Now the devres keeps
> only the card object pointer, not the card object itself, and the card
> object is unreferenced at both releases.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

My bad, scratch this mail; it was slipped into the submission from the
previously generated patch files...

Other 6 patches (with RFC prefix) are fine.


Takashi
diff mbox series

Patch

diff --git a/sound/core/init.c b/sound/core/init.c
index 7e7c4b8d4e11..22da438faf40 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -231,7 +231,7 @@  int snd_card_new(struct device *parent, int idx, const char *xid,
 
 	if (extra_size < 0)
 		extra_size = 0;
-	card = kzalloc(sizeof(*card) + extra_size, GFP_KERNEL);
+	card = snd_refmem_alloc(sizeof(*card) + extra_size, NULL);
 	if (!card)
 		return -ENOMEM;
 
@@ -246,7 +246,14 @@  EXPORT_SYMBOL(snd_card_new);
 
 static void __snd_card_release(struct device *dev, void *data)
 {
-	snd_card_free(data);
+	struct snd_card **card_p = data;
+	struct snd_card *card;
+
+	if (card_p) {
+		card = *card_p;
+		snd_card_free(card);
+		snd_refmem_put(card);
+	}
 }
 
 /**
@@ -279,21 +286,22 @@  int snd_devm_card_new(struct device *parent, int idx, const char *xid,
 		      struct snd_card **card_ret)
 {
 	struct snd_card *card;
+	struct snd_card **card_devres;
 	int err;
 
 	*card_ret = NULL;
-	card = devres_alloc(__snd_card_release, sizeof(*card) + extra_size,
-			    GFP_KERNEL);
-	if (!card)
+	card_devres = devres_alloc(__snd_card_release, sizeof(void *), GFP_KERNEL);
+	if (!card_devres)
 		return -ENOMEM;
-	card->managed = true;
-	err = snd_card_init(card, parent, idx, xid, module, extra_size);
-	if (err < 0) {
-		devres_free(card); /* in managed mode, we need to free manually */
-		return err;
-	}
+	devres_add(parent, card_devres);
 
-	devres_add(parent, card);
+	err = snd_card_new(parent, idx, xid, module, extra_size, &card);
+	if (err)
+		return err;
+
+	card->managed = true;
+	snd_refmem_get(card);
+	*card_devres = card;
 	*card_ret = card;
 	return 0;
 }
@@ -353,8 +361,7 @@  static int snd_card_init(struct snd_card *card, struct device *parent,
 		mutex_unlock(&snd_card_mutex);
 		dev_err(parent, "cannot find the slot for index %d (range 0-%i), error: %d\n",
 			 idx, snd_ecards_limit - 1, err);
-		if (!card->managed)
-			kfree(card); /* manually free here, as no destructor called */
+		snd_refmem_put(card); /* manually free here, as no destructor called */
 		return err;
 	}
 	set_bit(idx, snd_cards_lock);		/* lock it */
@@ -650,8 +657,7 @@  static int snd_card_do_free(struct snd_card *card)
 #endif
 	if (card->release_completion)
 		complete(card->release_completion);
-	if (!card->managed)
-		kfree(card);
+	snd_refmem_put(card);
 	return 0;
 }