* [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
@ 2023-03-02 16:36 Alvin Šipraga
2023-03-02 19:16 ` John Keeping
2023-03-03 0:51 ` Ruslan Bilovol
0 siblings, 2 replies; 5+ messages in thread
From: Alvin Šipraga @ 2023-03-02 16:36 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Greg Kroah-Hartman, Yadi Brar,
Jassi Brar, Felipe Balbi
Cc: alsa-devel, Alvin Šipraga, stable, linux-usb, linux-kernel
From: Alvin Šipraga <alsi@bang-olufsen.dk>
In the unbind callback for f_uac1 and f_uac2, a call to snd_card_free()
via g_audio_cleanup() will disconnect the card and then wait for all
resources to be released, which happens when the refcount falls to zero.
Since userspace can keep the refcount incremented by not closing the
relevant file descriptor, the call to unbind may block indefinitely.
This can cause a deadlock during reboot, as evidenced by the following
blocked task observed on my machine:
task:reboot state:D stack:0 pid:2827 ppid:569 flags:0x0000000c
Call trace:
__switch_to+0xc8/0x140
__schedule+0x2f0/0x7c0
schedule+0x60/0xd0
schedule_timeout+0x180/0x1d4
wait_for_completion+0x78/0x180
snd_card_free+0x90/0xa0
g_audio_cleanup+0x2c/0x64
afunc_unbind+0x28/0x60
...
kernel_restart+0x4c/0xac
__do_sys_reboot+0xcc/0x1ec
__arm64_sys_reboot+0x28/0x30
invoke_syscall+0x4c/0x110
...
The issue can also be observed by opening the card with arecord and
then stopping the process through the shell before unbinding:
# arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
Recording WAVE '/dev/null' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
^Z[1]+ Stopped arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
# echo gadget.0 > /sys/bus/gadget/drivers/configfs-gadget/unbind
(observe that the unbind command never finishes)
Fix the problem by using snd_card_free_when_closed() instead, which will
still disconnect the card as desired, but defer the task of freeing the
resources to the core once userspace closes its file descriptor.
Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
drivers/usb/gadget/function/u_audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index c1f62e91b012..4a42574b4a7f 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -1422,7 +1422,7 @@ void g_audio_cleanup(struct g_audio *g_audio)
uac = g_audio->uac;
card = uac->card;
if (card)
- snd_card_free(card);
+ snd_card_free_when_closed(card);
kfree(uac->p_prm.reqs);
kfree(uac->c_prm.reqs);
--
2.39.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
2023-03-02 16:36 [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind Alvin Šipraga
@ 2023-03-02 19:16 ` John Keeping
2023-03-03 0:51 ` Ruslan Bilovol
1 sibling, 0 replies; 5+ messages in thread
From: John Keeping @ 2023-03-02 19:16 UTC (permalink / raw)
To: Alvin Šipraga
Cc: Takashi Iwai, Greg Kroah-Hartman, Yadi Brar, Jassi Brar,
Felipe Balbi, alsa-devel, Alvin Šipraga, stable, linux-usb,
linux-kernel
On Thu, Mar 02, 2023 at 05:36:47PM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> In the unbind callback for f_uac1 and f_uac2, a call to snd_card_free()
> via g_audio_cleanup() will disconnect the card and then wait for all
> resources to be released, which happens when the refcount falls to zero.
> Since userspace can keep the refcount incremented by not closing the
> relevant file descriptor, the call to unbind may block indefinitely.
> This can cause a deadlock during reboot, as evidenced by the following
> blocked task observed on my machine:
>
> task:reboot state:D stack:0 pid:2827 ppid:569 flags:0x0000000c
> Call trace:
> __switch_to+0xc8/0x140
> __schedule+0x2f0/0x7c0
> schedule+0x60/0xd0
> schedule_timeout+0x180/0x1d4
> wait_for_completion+0x78/0x180
> snd_card_free+0x90/0xa0
> g_audio_cleanup+0x2c/0x64
> afunc_unbind+0x28/0x60
> ...
> kernel_restart+0x4c/0xac
> __do_sys_reboot+0xcc/0x1ec
> __arm64_sys_reboot+0x28/0x30
> invoke_syscall+0x4c/0x110
> ...
>
> The issue can also be observed by opening the card with arecord and
> then stopping the process through the shell before unbinding:
>
> # arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> Recording WAVE '/dev/null' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
> ^Z[1]+ Stopped arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> # echo gadget.0 > /sys/bus/gadget/drivers/configfs-gadget/unbind
> (observe that the unbind command never finishes)
>
> Fix the problem by using snd_card_free_when_closed() instead, which will
> still disconnect the card as desired, but defer the task of freeing the
> resources to the core once userspace closes its file descriptor.
>
> Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: John Keeping <john@metanate.com>
> ---
> drivers/usb/gadget/function/u_audio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c1f62e91b012..4a42574b4a7f 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -1422,7 +1422,7 @@ void g_audio_cleanup(struct g_audio *g_audio)
> uac = g_audio->uac;
> card = uac->card;
> if (card)
> - snd_card_free(card);
> + snd_card_free_when_closed(card);
>
> kfree(uac->p_prm.reqs);
> kfree(uac->c_prm.reqs);
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
@ 2023-03-02 19:16 ` John Keeping
0 siblings, 0 replies; 5+ messages in thread
From: John Keeping @ 2023-03-02 19:16 UTC (permalink / raw)
To: Alvin Šipraga
Cc: Jaroslav Kysela, Takashi Iwai, Greg Kroah-Hartman, Yadi Brar,
Jassi Brar, Felipe Balbi, alsa-devel, Alvin Šipraga, stable,
linux-usb, linux-kernel
On Thu, Mar 02, 2023 at 05:36:47PM +0100, Alvin Šipraga wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> In the unbind callback for f_uac1 and f_uac2, a call to snd_card_free()
> via g_audio_cleanup() will disconnect the card and then wait for all
> resources to be released, which happens when the refcount falls to zero.
> Since userspace can keep the refcount incremented by not closing the
> relevant file descriptor, the call to unbind may block indefinitely.
> This can cause a deadlock during reboot, as evidenced by the following
> blocked task observed on my machine:
>
> task:reboot state:D stack:0 pid:2827 ppid:569 flags:0x0000000c
> Call trace:
> __switch_to+0xc8/0x140
> __schedule+0x2f0/0x7c0
> schedule+0x60/0xd0
> schedule_timeout+0x180/0x1d4
> wait_for_completion+0x78/0x180
> snd_card_free+0x90/0xa0
> g_audio_cleanup+0x2c/0x64
> afunc_unbind+0x28/0x60
> ...
> kernel_restart+0x4c/0xac
> __do_sys_reboot+0xcc/0x1ec
> __arm64_sys_reboot+0x28/0x30
> invoke_syscall+0x4c/0x110
> ...
>
> The issue can also be observed by opening the card with arecord and
> then stopping the process through the shell before unbinding:
>
> # arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> Recording WAVE '/dev/null' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
> ^Z[1]+ Stopped arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> # echo gadget.0 > /sys/bus/gadget/drivers/configfs-gadget/unbind
> (observe that the unbind command never finishes)
>
> Fix the problem by using snd_card_free_when_closed() instead, which will
> still disconnect the card as desired, but defer the task of freeing the
> resources to the core once userspace closes its file descriptor.
>
> Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: John Keeping <john@metanate.com>
> ---
> drivers/usb/gadget/function/u_audio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c1f62e91b012..4a42574b4a7f 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -1422,7 +1422,7 @@ void g_audio_cleanup(struct g_audio *g_audio)
> uac = g_audio->uac;
> card = uac->card;
> if (card)
> - snd_card_free(card);
> + snd_card_free_when_closed(card);
>
> kfree(uac->p_prm.reqs);
> kfree(uac->c_prm.reqs);
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
2023-03-02 16:36 [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind Alvin Šipraga
@ 2023-03-03 0:51 ` Ruslan Bilovol
2023-03-03 0:51 ` Ruslan Bilovol
1 sibling, 0 replies; 5+ messages in thread
From: Ruslan Bilovol @ 2023-03-03 0:51 UTC (permalink / raw)
To: Alvin Šipraga
Cc: Jaroslav Kysela, Takashi Iwai, Greg Kroah-Hartman, Yadi Brar,
Jassi Brar, Felipe Balbi, alsa-devel, Alvin Šipraga, stable,
linux-usb, linux-kernel
On Thu, Mar 2, 2023 at 11:39 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
>
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> In the unbind callback for f_uac1 and f_uac2, a call to snd_card_free()
> via g_audio_cleanup() will disconnect the card and then wait for all
> resources to be released, which happens when the refcount falls to zero.
> Since userspace can keep the refcount incremented by not closing the
> relevant file descriptor, the call to unbind may block indefinitely.
> This can cause a deadlock during reboot, as evidenced by the following
> blocked task observed on my machine:
>
> task:reboot state:D stack:0 pid:2827 ppid:569 flags:0x0000000c
> Call trace:
> __switch_to+0xc8/0x140
> __schedule+0x2f0/0x7c0
> schedule+0x60/0xd0
> schedule_timeout+0x180/0x1d4
> wait_for_completion+0x78/0x180
> snd_card_free+0x90/0xa0
> g_audio_cleanup+0x2c/0x64
> afunc_unbind+0x28/0x60
> ...
> kernel_restart+0x4c/0xac
> __do_sys_reboot+0xcc/0x1ec
> __arm64_sys_reboot+0x28/0x30
> invoke_syscall+0x4c/0x110
> ...
>
> The issue can also be observed by opening the card with arecord and
> then stopping the process through the shell before unbinding:
>
> # arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> Recording WAVE '/dev/null' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
> ^Z[1]+ Stopped arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> # echo gadget.0 > /sys/bus/gadget/drivers/configfs-gadget/unbind
> (observe that the unbind command never finishes)
>
> Fix the problem by using snd_card_free_when_closed() instead, which will
> still disconnect the card as desired, but defer the task of freeing the
> resources to the core once userspace closes its file descriptor.
It seems nobody has tested that use-case before. Thank you for fixing it
Reviewed-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>
> Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
> drivers/usb/gadget/function/u_audio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c1f62e91b012..4a42574b4a7f 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -1422,7 +1422,7 @@ void g_audio_cleanup(struct g_audio *g_audio)
> uac = g_audio->uac;
> card = uac->card;
> if (card)
> - snd_card_free(card);
> + snd_card_free_when_closed(card);
>
> kfree(uac->p_prm.reqs);
> kfree(uac->c_prm.reqs);
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
@ 2023-03-03 0:51 ` Ruslan Bilovol
0 siblings, 0 replies; 5+ messages in thread
From: Ruslan Bilovol @ 2023-03-03 0:51 UTC (permalink / raw)
To: Alvin Šipraga
Cc: Takashi Iwai, Greg Kroah-Hartman, Yadi Brar, Jassi Brar,
Felipe Balbi, alsa-devel, Alvin Šipraga, stable, linux-usb,
linux-kernel
On Thu, Mar 2, 2023 at 11:39 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
>
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> In the unbind callback for f_uac1 and f_uac2, a call to snd_card_free()
> via g_audio_cleanup() will disconnect the card and then wait for all
> resources to be released, which happens when the refcount falls to zero.
> Since userspace can keep the refcount incremented by not closing the
> relevant file descriptor, the call to unbind may block indefinitely.
> This can cause a deadlock during reboot, as evidenced by the following
> blocked task observed on my machine:
>
> task:reboot state:D stack:0 pid:2827 ppid:569 flags:0x0000000c
> Call trace:
> __switch_to+0xc8/0x140
> __schedule+0x2f0/0x7c0
> schedule+0x60/0xd0
> schedule_timeout+0x180/0x1d4
> wait_for_completion+0x78/0x180
> snd_card_free+0x90/0xa0
> g_audio_cleanup+0x2c/0x64
> afunc_unbind+0x28/0x60
> ...
> kernel_restart+0x4c/0xac
> __do_sys_reboot+0xcc/0x1ec
> __arm64_sys_reboot+0x28/0x30
> invoke_syscall+0x4c/0x110
> ...
>
> The issue can also be observed by opening the card with arecord and
> then stopping the process through the shell before unbinding:
>
> # arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> Recording WAVE '/dev/null' : Signed 32 bit Little Endian, Rate 48000 Hz, Stereo
> ^Z[1]+ Stopped arecord -D hw:UAC2Gadget -f S32_LE -c 2 -r 48000 /dev/null
> # echo gadget.0 > /sys/bus/gadget/drivers/configfs-gadget/unbind
> (observe that the unbind command never finishes)
>
> Fix the problem by using snd_card_free_when_closed() instead, which will
> still disconnect the card as desired, but defer the task of freeing the
> resources to the core once userspace closes its file descriptor.
It seems nobody has tested that use-case before. Thank you for fixing it
Reviewed-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>
> Fixes: 132fcb460839 ("usb: gadget: Add Audio Class 2.0 Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
> drivers/usb/gadget/function/u_audio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index c1f62e91b012..4a42574b4a7f 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -1422,7 +1422,7 @@ void g_audio_cleanup(struct g_audio *g_audio)
> uac = g_audio->uac;
> card = uac->card;
> if (card)
> - snd_card_free(card);
> + snd_card_free_when_closed(card);
>
> kfree(uac->p_prm.reqs);
> kfree(uac->c_prm.reqs);
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-03 0:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-02 16:36 [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind Alvin Šipraga
2023-03-02 19:16 ` John Keeping
2023-03-02 19:16 ` John Keeping
2023-03-03 0:51 ` Ruslan Bilovol
2023-03-03 0:51 ` Ruslan Bilovol
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.