Re: [PATCH] usb: fix a task hung in snd_card_free

[Takashi Iwai <tiwai@xxxxxxx>]

On Wed, 06 Nov 2024 03:15:49 +0100,
Edward Adam Davis wrote:
> 
> task 1: snd ctrl will add card_dev ref count and can't call close to dec it,
>         it is blocked waiting for task 2 to release the USB dev lock.
> 
> task 2: usb dev lock has been locked by hung task (here is usb_disconnect),
>         it is hung waiting for task 1 to exit and release card_dev.
> 
> Adjust the USB lock acquisition method to non-blocking in ioctl to avoid
> hang when the USB connection is closed.

I'm afraid that this change would break things too badly.
i.e. changing the blocking behavior to non-blocking is no-go.

> Reported-and-tested-by: syzbot+73582d08864d8268b6fd@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=73582d08864d8268b6fd

This particular syzkaller entry can be fixed rather by replacing
snd_card_free() in snd_usx2y_disconnect() with
snd_card_free_when_closed() like other USB audio drivers, something
like below.

Judging from the git log, it had been with snd_card_free_in_thread(),
but was switch to snd_card_free() around year 2005.  Meanwhile the
handling of async card release got improved, and it's very likely OK
to use snd_card_free_when_closed() there with the recent kernel.


thanks,

Takashi

-- 8< --
--- a/sound/usb/usx2y/usbusx2y.c
+++ b/sound/usb/usx2y/usbusx2y.c
@@ -422,7 +422,7 @@ static void snd_usx2y_disconnect(struct usb_interface *intf)
 	}
 	if (usx2y->us428ctls_sharedmem)
 		wake_up(&usx2y->us428ctls_wait_queue_head);
-	snd_card_free(card);
+	snd_card_free_when_closed(card);
 }
 
 static int snd_usx2y_probe(struct usb_interface *intf,