[PATCH] ALSA: hda: intel: Don't free card in the probe work

[PATCH] ALSA: hda: intel: Don't free card in the probe work

[Ali Ahmet Memiş]

If the delayed probe fails, azx_probe_work() frees the card and clears
the drvdata. azx_remove() reads the card before it waits for the probe
work with cancel_delayed_work_sync() so when the device is removed while
the probe work is running, it frees the card again after the work has
already freed it.

Leave the card to azx_remove() and only mark the probe as failed in the
work as is done in the vga_switcheroo path.

Fixes: 39173303c838 ("ALSA: hda: Free card instance properly at probe errors")
Reported-by: syzbot+40b06a86af225236fdb4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=40b06a86af225236fdb4
Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>
---
With this, azx_probe_continue() is always called with free_on_error
false.  So the parameter is no longer needed.  I can drop it in v2 in
the same commit if that would be OK.

 sound/hda/controllers/intel.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/hda/controllers/intel.c b/sound/hda/controllers/intel.c
index 2b5b5d555f2c..5db3d37f93fe 100644
--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -1737,7 +1737,11 @@ static void azx_check_snoop_available(struct azx *chip)
 static void azx_probe_work(struct work_struct *work)
 {
 	struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
-	azx_probe_continue(&hda->chip, true);
+
+	if (azx_probe_continue(&hda->chip, false) < 0) {
+		hda->init_failed = 1;
+		complete_all(&hda->probe_wait);
+	}
 }
 
 static int default_bdl_pos_adj(struct azx *chip)
-- 
2.55.0

Re: [PATCH] ALSA: hda: intel: Don't free card in the probe work

[Takashi Iwai]

On Sun, 11 Oct 2026 22:06:27 +0200,
Ali Ahmet Memiş wrote:
> 
> If the delayed probe fails, azx_probe_work() frees the card and clears
> the drvdata. azx_remove() reads the card before it waits for the probe
> work with cancel_delayed_work_sync() so when the device is removed while
> the probe work is running, it frees the card again after the work has
> already freed it.
> 
> Leave the card to azx_remove() and only mark the probe as failed in the
> work as is done in the vga_switcheroo path.
> 
> Fixes: 39173303c838 ("ALSA: hda: Free card instance properly at probe errors")
> Reported-by: syzbot+40b06a86af225236fdb4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=40b06a86af225236fdb4
> Signed-off-by: Ali Ahmet Memiş <aliamemis@disroot.org>

I believe this may change the behavior visible on user-space, and it's
not what we want.

For avoiding the card free, can it be the refcounting like below
instead?


thanks,

Takashi

--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -1737,7 +1737,11 @@ static void azx_check_snoop_available(struct azx *chip)
 static void azx_probe_work(struct work_struct *work)
 {
 	struct hda_intel *hda = container_of(work, struct hda_intel, probe_work.work);
+	struct snd_card *card = hda->core.card;
+
+	get_device(&card->card_dev);
 	azx_probe_continue(&hda->chip, true);
+	put_device(&card->card_dev);
 }
 
 static int default_bdl_pos_adj(struct azx *chip)

Re: [PATCH] ALSA: hda: intel: Don't free card in the probe work

[Ali Ahmet Memiş]

On Mon, 12 Oct 2026 09:19:36 +0200, Takashi Iwai wrote:
> For avoiding the card free, can it be the refcounting like below
> instead?

I tried it in QEMU and it hangs on every failed delayed probe even
without a remove.  snd_card_free() waits for the release of card_dev,
but the reference taken in azx_probe_work() is dropped only after
azx_probe_continue() returns, so the work never finishes:

kworker/1:1+events:
[<0>] snd_card_free+0x107/0x160
[<0>] azx_probe_work+0x57d/0x1fa0
[<0>] process_one_work+0x937/0x1800
[<0>] worker_thread+0x4ba/0xd20
[<0>] kthread+0x331/0x420

It also doesn't help azx_remove() which reads the card without a
reference before cancel_delayed_work_sync() and frees it again after
the work.

Which user-visible change did you mean?  The card now stays until the
device is removed instead of being freed in the work.

Re: [PATCH] ALSA: hda: intel: Don't free card in the probe work

[Takashi Iwai]

On Mon, 12 Oct 2026 16:35:05 +0200,
Ali Ahmet Memiş wrote:
> 
> On Mon, 12 Oct 2026 09:19:36 +0200, Takashi Iwai wrote:
> > For avoiding the card free, can it be the refcounting like below
> > instead?
> 
> I tried it in QEMU and it hangs on every failed delayed probe even
> without a remove.  snd_card_free() waits for the release of card_dev,
> but the reference taken in azx_probe_work() is dropped only after
> azx_probe_continue() returns, so the work never finishes:
> 
> kworker/1:1+events:
> [<0>] snd_card_free+0x107/0x160
> [<0>] azx_probe_work+0x57d/0x1fa0
> [<0>] process_one_work+0x937/0x1800
> [<0>] worker_thread+0x4ba/0xd20
> [<0>] kthread+0x331/0x420
> 
> It also doesn't help azx_remove() which reads the card without a
> reference before cancel_delayed_work_sync() and frees it again after
> the work.

OK, that's a problem, indeed.  How about replacing snd_card_free()
with *_when_closed() version that doesn't block in addition?

--- a/sound/hda/controllers/intel.c
+++ b/sound/hda/controllers/intel.c
@@ -2450,7 +2450,7 @@ static int azx_probe_continue(struct azx *chip, bool free_on_error)
 			dev_warn(&pci->dev, "GPU sound probed, but not operational: please add a quirk to driver_denylist\n");
 		if (free_on_error) {
 			pci_set_drvdata(pci, NULL);
-			snd_card_free(chip->card);
+			snd_card_free_when_closed(chip->card);
 		}
 		return err;
 	}

> Which user-visible change did you mean?  The card now stays until the
> device is removed instead of being freed in the work.

Staying the card means that the card slot will be kept, then if you
probe another card afterwards, it'll end up with assignment on another
card slot.


thanks,

Takashi

Re: [PATCH] ALSA: hda: intel: Don't free card in the probe work

[Ali Ahmet Memiş]

On Mon, 12 Oct 2026 16:57:37 +0200, Takashi Iwai wrote:
> OK, that's a problem, indeed.  How about replacing snd_card_free()
> with *_when_closed() version that doesn't block in addition?

The work doesn't hang anymore but removing the device while the probe
work is running hits the same UAF on the first try.  azx_remove() still
gets the card before cancel_delayed_work_sync() and the put_device() in
the work frees it:

BUG: KASAN: slab-use-after-free in snd_card_free+0x148/0x160
Read of size 1 at addr ffff888007724852 by task sh/97
Call Trace:
 snd_card_free+0x148/0x160
 pci_device_remove+0xb6/0x1d0
 device_release_driver_internal+0x38d/0x540
 pci_stop_bus_device+0x122/0x170
 ...
Freed by task 30:
 kfree+0x1e8/0x5e0
 device_release+0xcf/0x3c0
 kobject_put+0x169/0x320
 azx_probe_work+0x495/0x1fa0
 process_one_work+0x937/0x1800

> Staying the card means that the card slot will be kept, then if you
> probe another card afterwards, it'll end up with assignment on another
> card slot.

What about calling snd_card_disconnect() in the work on failure?  It
releases the slot right away and the card is still freed only in
azx_remove():

	if (azx_probe_continue(&hda->chip, false) < 0) {
		hda->init_failed = 1;
		complete_all(&hda->probe_wait);
		snd_card_disconnect(hda->chip.card);
	}

With that, snd-dummy loaded after a failed probe gets card 0 and 20
rounds of the remove race were clean with KASAN.  I can send v2 if this
looks fine.

Re: [PATCH] ALSA: hda: intel: Don't free card in the probe work

[Takashi Iwai]

On Mon, 12 Oct 2026 17:09:41 +0200,
Ali Ahmet Memiş wrote:
> 
> On Mon, 12 Oct 2026 16:57:37 +0200, Takashi Iwai wrote:
> > OK, that's a problem, indeed.  How about replacing snd_card_free()
> > with *_when_closed() version that doesn't block in addition?
> 
> The work doesn't hang anymore but removing the device while the probe
> work is running hits the same UAF on the first try.  azx_remove() still
> gets the card before cancel_delayed_work_sync() and the put_device() in
> the work frees it:
> 
> BUG: KASAN: slab-use-after-free in snd_card_free+0x148/0x160
> Read of size 1 at addr ffff888007724852 by task sh/97
> Call Trace:
>  snd_card_free+0x148/0x160
>  pci_device_remove+0xb6/0x1d0
>  device_release_driver_internal+0x38d/0x540
>  pci_stop_bus_device+0x122/0x170
>  ...
> Freed by task 30:
>  kfree+0x1e8/0x5e0
>  device_release+0xcf/0x3c0
>  kobject_put+0x169/0x320
>  azx_probe_work+0x495/0x1fa0
>  process_one_work+0x937/0x1800
> 
> > Staying the card means that the card slot will be kept, then if you
> > probe another card afterwards, it'll end up with assignment on another
> > card slot.
> 
> What about calling snd_card_disconnect() in the work on failure?  It
> releases the slot right away and the card is still freed only in
> azx_remove():
> 
> 	if (azx_probe_continue(&hda->chip, false) < 0) {
> 		hda->init_failed = 1;
> 		complete_all(&hda->probe_wait);
> 		snd_card_disconnect(hda->chip.card);
> 	}
> 
> With that, snd-dummy loaded after a failed probe gets card 0 and 20
> rounds of the remove race were clean with KASAN.  I can send v2 if this
> looks fine.

Yeah, that sounds like a plan.
Care to resubmit a fix with that approach?


thanks,

Takashi