MIME-Version: 1.0
References: <20230302163648.3349669-1-alvin@pqrs.dk>
In-Reply-To: <20230302163648.3349669-1-alvin@pqrs.dk>
From:   Ruslan Bilovol <ruslan.bilovol@gmail.com>
Date:   Thu, 2 Mar 2023 19:51:02 -0500
Message-ID: <CAB=otbTiCd0RTbJQxVrp1BB=MVtH-U70eM0fTMHdFH33bC00HA@mail.gmail.com>
Subject: Re: [PATCH] usb: gadget: u_audio: don't let userspace block driver unbind
To:     =?UTF-8?Q?Alvin_=C5=A0ipraga?= <alvin@pqrs.dk>
Cc:     Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Yadi Brar <yadi.brar01@gmail.com>,
        Jassi Brar <jaswinder.singh@linaro.org>,
        Felipe Balbi <balbi@ti.com>, alsa-devel@alsa-project.org,
        =?UTF-8?Q?Alvin_=C5=A0ipraga?= <alsi@bang-olufsen.dk>,
        stable@vger.kernel.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 2, 2023 at 11:39=E2=80=AFAM Alvin =C5=A0ipraga <alvin@pqrs.dk> =
wrote:
>
> From: Alvin =C5=A0ipraga <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 =C5=A0ipraga <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/f=
unction/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 =3D g_audio->uac;
>         card =3D 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
>