[PATCH] usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio

[PATCH] usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

The OUT endpoint completion (f_audio_out_ep_complete) and the set_alt path
schedule audio->playback_work, whose handler (f_audio_playback_work)
dereferences the f_audio (audio->lock, audio->play_queue, audio->card).
On teardown f_audio_free() frees the f_audio with kfree(audio) but never
cancels playback_work -- f_audio_disable() is empty and nothing else
drains it. A playback_work still pending when the function is freed runs
against freed memory, a use-after-free.

Cancel playback_work in f_audio_free() before kfree(audio). By then the
function is unbound and the OUT endpoint is disabled, so the work cannot
be re-queued and cancel_work_sync() is sufficient.

This was reproduced with KASAN as a slab-use-after-free in
u_audio_playback(), reached from f_audio_playback_work(), under dummy_hcd:
with a UAC1 playback stream active, unlinking the configfs function frees
the f_audio in f_audio_free() while the work is still in flight. A delay
was added to the gadget streaming path to make the schedule-vs-free race
land deterministically; the missing cancel is present regardless. With
this patch applied the splat no longer occurs.

Fixes: d355339eecd9 ("usb: gadget: function: make current f_uac1 implementation legacy")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/usb/gadget/function/f_uac1_legacy.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index 7c8d60c..2dca5cd 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -945,6 +945,12 @@ static void f_audio_free(struct usb_function *f)
 
 	gaudio_cleanup(&audio->card);
 	opts = container_of(f->fi, struct f_uac1_legacy_opts, func_inst);
+	/*
+	 * The function is already unbound, so the OUT endpoint can no longer
+	 * queue playback_work. Drain any work still pending before the
+	 * containing f_audio is freed, otherwise it runs against freed memory.
+	 */
+	cancel_work_sync(&audio->playback_work);
 	kfree(audio);
 	mutex_lock(&opts->lock);
 	--opts->refcnt;
-- 
2.43.0

Re: [PATCH] usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [Medium] usb: gadget: f_uac1_legacy: cancel_work_sync() after
  gaudio_cleanup()
- [Medium] usb: gadget: f_uac1_legacy: play_queue buffer leak in
  f_audio_free()
- [Low] usb: gadget: f_uac1_legacy: Fixes tag cites file rename commit
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] usb: gadget: f_uac1_legacy: use-after-free in
  f_audio_complete() due to undisabled out_ep [1]
--

commit 12cbf9965abf5188436aa00e354e601adc2d615b
Author: Hyunjung Ko <hj351016@gmail.com>

> usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio
>
> The OUT endpoint completion (f_audio_out_ep_complete) and the set_alt path
> schedule audio->playback_work, whose handler (f_audio_playback_work)
> dereferences the f_audio (audio->lock, audio->play_queue, audio->card).
> On teardown f_audio_free() frees the f_audio with kfree(audio) but never
> cancels playback_work -- f_audio_disable() is empty and nothing else
> drains it. A playback_work still pending when the function is freed runs
> against freed memory, a use-after-free.
>
> Cancel playback_work in f_audio_free() before kfree(audio). By then the
> function is unbound and the OUT endpoint is disabled, so the work cannot
> be re-queued and cancel_work_sync() is sufficient.
>
> This was reproduced with KASAN as a slab-use-after-free in
> u_audio_playback(), reached from f_audio_playback_work(), under dummy_hcd:
> with a UAC1 playback stream active, unlinking the configfs function frees
> the f_audio in f_audio_free() while the work is still in flight. A delay
> was added to the gadget streaming path to make the schedule-vs-free race
> land deterministically; the missing cancel is present regardless. With
> this patch applied the splat no longer occurs.
>
> Fixes: d355339eecd9 ("usb: gadget: function: make current f_uac1 implementation legacy")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>

[Severity: Low]
Commit d355339eecd9 ("usb: gadget: function: make current f_uac1
implementation legacy") only renamed f_uac1.c to f_uac1_legacy.c.  Should
the Fixes: tag point to f3a3406b3f56 ("usb: gadget: f_uac1: convert to new
function interface with backward compatibility"), which introduced
f_audio_free() without cancelling playback_work?

> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index 7c8d60c7e87d1..2dca5cda8cfd7 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -945,6 +945,12 @@ static void f_audio_free(struct usb_function *f)
>  
>  	gaudio_cleanup(&audio->card);
>  	opts = container_of(f->fi, struct f_uac1_legacy_opts, func_inst);
> +	/*
> +	 * The function is already unbound, so the OUT endpoint can no longer
> +	 * queue playback_work. Drain any work still pending before the
> +	 * containing f_audio is freed, otherwise it runs against freed memory.
> +	 */
> +	cancel_work_sync(&audio->playback_work);
>  	kfree(audio);

[Severity: Medium]
Should cancel_work_sync(&audio->playback_work) be called before
gaudio_cleanup(&audio->card)?

f_audio_playback_work() calls u_audio_playback(&audio->card, ...), which
reads substream->runtime from card->playback.substream and loops back to
try_again when snd_pcm_kernel_write() fails.  Calling gaudio_cleanup()
first closes card->playback.filp while playback_work may still be running,
which can free substream->runtime underneath u_audio_playback() or leave it
retrying in try_again while cancel_work_sync() waits.

[Severity: Medium]
Does cancelling audio->playback_work here leak buffers left on
audio->play_queue (as well as audio->copy_buf)?

f_audio_out_ep_complete() and f_audio_set_alt() append each allocated
struct f_audio_buf to audio->play_queue and schedule audio->playback_work,
and f_audio_playback_work() is the only place that dequeues and frees them
with f_audio_buffer_free().  When cancel_work_sync() cancels a pending work
item, or when more than one buffer is queued, the remaining buffers on
audio->play_queue are never freed before kfree(audio).

[Severity: Critical]
This problem wasn't introduced by this patch, but nothing disables
audio->out_ep before f_audio_free() runs [1], so can
f_audio_complete() still race with teardown and use audio after free?

When the host selects alternate setting 1, f_audio_set_alt() enables
audio->out_ep and queues usb_request objects with req->context = audio and
req->complete = f_audio_complete, while f_audio_disable() is a no-op and
neither f_audio_set_alt(), f_audio_unbind(), nor f_audio_free() calls
usb_ep_disable(audio->out_ep):

CPU0                                  CPU1
f_audio_free()
  cancel_work_sync(&audio->playback_work)
  kfree(audio)
                                      f_audio_complete()
                                        audio = req->context
                                        out_ep = audio->out_ep <-- use-after-free
                                        f_audio_out_ep_complete()
                                          schedule_work(&audio->playback_work)

During gadget_unbind_driver(), udc->driver->unbind() runs before
usb_gadget_udc_stop_locked() gives back pending requests on the enabled
endpoint with -ESHUTDOWN, so f_audio_complete() can also be invoked after
f_audio is freed.

[1] https://sashiko.dev/#/bug/linux-7598f007-24ed-4b73-bc9a-a11e4ec9838e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108075116.251501-1-hj351016@gmail.com?part=1

Re: [PATCH] usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio

[khj]

Thanks -- all four are valid, and the Critical is the real root cause.

v1 only cancelled playback_work, which is necessary but not sufficient:
as you note, f_audio_disable() is empty, so out_ep stays enabled and
f_audio_complete() can run against freed memory during unbind regardless
of the work. v1 also cancelled after gaudio_cleanup() and leaked the
play_queue buffers.

v2 implements f_audio_disable() to quiesce the function properly: disable
the OUT endpoint (which gives back the outstanding requests, so
f_audio_complete() stops touching the freed f_audio and no new work is
queued), then cancel_work_sync() the playback work, then free the buffers
left on play_queue/copy_buf. That addresses the Critical, the ordering,
and the leak in one place. I've also corrected the Fixes: tag to
f3a3406b3f56 per your note.

2026년 11월 8일 (일) 오후 5:13, <sashiko-bot@kernel.org>님이 작성:
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issues to consider:
> - [Medium] usb: gadget: f_uac1_legacy: cancel_work_sync() after
>   gaudio_cleanup()
> - [Medium] usb: gadget: f_uac1_legacy: play_queue buffer leak in
>   f_audio_free()
> - [Low] usb: gadget: f_uac1_legacy: Fixes tag cites file rename commit
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [Critical] usb: gadget: f_uac1_legacy: use-after-free in
>   f_audio_complete() due to undisabled out_ep [1]
> --
>
> commit 12cbf9965abf5188436aa00e354e601adc2d615b
> Author: Hyunjung Ko <hj351016@gmail.com>
>
> > usb: gadget: f_uac1_legacy: cancel playback_work before freeing f_audio
> >
> > The OUT endpoint completion (f_audio_out_ep_complete) and the set_alt path
> > schedule audio->playback_work, whose handler (f_audio_playback_work)
> > dereferences the f_audio (audio->lock, audio->play_queue, audio->card).
> > On teardown f_audio_free() frees the f_audio with kfree(audio) but never
> > cancels playback_work -- f_audio_disable() is empty and nothing else
> > drains it. A playback_work still pending when the function is freed runs
> > against freed memory, a use-after-free.
> >
> > Cancel playback_work in f_audio_free() before kfree(audio). By then the
> > function is unbound and the OUT endpoint is disabled, so the work cannot
> > be re-queued and cancel_work_sync() is sufficient.
> >
> > This was reproduced with KASAN as a slab-use-after-free in
> > u_audio_playback(), reached from f_audio_playback_work(), under dummy_hcd:
> > with a UAC1 playback stream active, unlinking the configfs function frees
> > the f_audio in f_audio_free() while the work is still in flight. A delay
> > was added to the gadget streaming path to make the schedule-vs-free race
> > land deterministically; the missing cancel is present regardless. With
> > this patch applied the splat no longer occurs.
> >
> > Fixes: d355339eecd9 ("usb: gadget: function: make current f_uac1 implementation legacy")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
>
> [Severity: Low]
> Commit d355339eecd9 ("usb: gadget: function: make current f_uac1
> implementation legacy") only renamed f_uac1.c to f_uac1_legacy.c.  Should
> the Fixes: tag point to f3a3406b3f56 ("usb: gadget: f_uac1: convert to new
> function interface with backward compatibility"), which introduced
> f_audio_free() without cancelling playback_work?
>
> > diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> > index 7c8d60c7e87d1..2dca5cda8cfd7 100644
> > --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> > +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> > @@ -945,6 +945,12 @@ static void f_audio_free(struct usb_function *f)
> >
> >       gaudio_cleanup(&audio->card);
> >       opts = container_of(f->fi, struct f_uac1_legacy_opts, func_inst);
> > +     /*
> > +      * The function is already unbound, so the OUT endpoint can no longer
> > +      * queue playback_work. Drain any work still pending before the
> > +      * containing f_audio is freed, otherwise it runs against freed memory.
> > +      */
> > +     cancel_work_sync(&audio->playback_work);
> >       kfree(audio);
>
> [Severity: Medium]
> Should cancel_work_sync(&audio->playback_work) be called before
> gaudio_cleanup(&audio->card)?
>
> f_audio_playback_work() calls u_audio_playback(&audio->card, ...), which
> reads substream->runtime from card->playback.substream and loops back to
> try_again when snd_pcm_kernel_write() fails.  Calling gaudio_cleanup()
> first closes card->playback.filp while playback_work may still be running,
> which can free substream->runtime underneath u_audio_playback() or leave it
> retrying in try_again while cancel_work_sync() waits.
>
> [Severity: Medium]
> Does cancelling audio->playback_work here leak buffers left on
> audio->play_queue (as well as audio->copy_buf)?
>
> f_audio_out_ep_complete() and f_audio_set_alt() append each allocated
> struct f_audio_buf to audio->play_queue and schedule audio->playback_work,
> and f_audio_playback_work() is the only place that dequeues and frees them
> with f_audio_buffer_free().  When cancel_work_sync() cancels a pending work
> item, or when more than one buffer is queued, the remaining buffers on
> audio->play_queue are never freed before kfree(audio).
>
> [Severity: Critical]
> This problem wasn't introduced by this patch, but nothing disables
> audio->out_ep before f_audio_free() runs [1], so can
> f_audio_complete() still race with teardown and use audio after free?
>
> When the host selects alternate setting 1, f_audio_set_alt() enables
> audio->out_ep and queues usb_request objects with req->context = audio and
> req->complete = f_audio_complete, while f_audio_disable() is a no-op and
> neither f_audio_set_alt(), f_audio_unbind(), nor f_audio_free() calls
> usb_ep_disable(audio->out_ep):
>
> CPU0                                  CPU1
> f_audio_free()
>   cancel_work_sync(&audio->playback_work)
>   kfree(audio)
>                                       f_audio_complete()
>                                         audio = req->context
>                                         out_ep = audio->out_ep <-- use-after-free
>                                         f_audio_out_ep_complete()
>                                           schedule_work(&audio->playback_work)
>
> During gadget_unbind_driver(), udc->driver->unbind() runs before
> usb_gadget_udc_stop_locked() gives back pending requests on the enabled
> endpoint with -ESHUTDOWN, so f_audio_complete() can also be invoked after
> f_audio is freed.
>
> [1] https://sashiko.dev/#/bug/linux-7598f007-24ed-4b73-bc9a-a11e4ec9838e
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261108075116.251501-1-hj351016@gmail.com?part=1