linux-kernel - Re: [PATCH] usb: fix a task hung in snd_card

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_F41B22255CBA04BB3B33319E461BFF4B3708@qq.com>
Date: Wed, 13 Nov 2024 09:48:49 +0800
From: Edward Adam Davis <eadavis@...com>
To: tiwai@...e.de
Cc: eadavis@...com,
	linux-kernel@...r.kernel.org,
	linux-sound@...r.kernel.org,
	perex@...ex.cz,
	syzbot+73582d08864d8268b6fd@...kaller.appspotmail.com,
	syzkaller-bugs@...glegroups.com,
	tiwai@...e.com
Subject: Re: [PATCH] usb: fix a task hung in snd_card_free

On Tue, 12 Nov 2024 17:04:04 +0100, Takashi Iwai wrote:
> 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@...kaller.appspotmail.com
> > 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.
The snd_card instance will be released in snd_card_do_free().
So, if snd_card_free_when_closed() is used to replace snd_card_free(), who will release the snd_card instance?

BR,
Edward
> 
> 
> 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,