[PATCH v3] usb: gadget: f_uac1_legacy: disable OUT ep and drain work on teardown

[PATCH v3] usb: gadget: f_uac1_legacy: disable OUT ep and drain work on teardown

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

f_uac1_legacy enables the OUT endpoint in f_audio_set_alt() and queues
usb_requests whose completion handler f_audio_complete() dereferences the
f_audio (req->context) and audio->out_ep, and which schedules
audio->playback_work. However f_audio_disable() is empty and nothing
disables the endpoint or drains the work on teardown, so:

 - f_audio_complete() can run after kfree(audio) (the UDC gives back
   pending requests on the still-enabled endpoint during unbind), using
   freed memory -- a use-after-free; and
 - a playback_work still pending runs f_audio_playback_work() against the
   freed f_audio -- a second use-after-free.

Disable the OUT endpoint in f_audio_disable(): the UDC then gives back the
outstanding requests, so f_audio_complete() stops touching @audio and no
new playback_work is queued. f_audio_disable() runs in atomic context
(reset_config() holds cdev->lock), and usb_ep_disable() is safe there.

Drain the work from f_audio_free() instead, where sleeping is allowed:
cancel_work_sync(&audio->playback_work) before gaudio_cleanup() tears down
audio->card (which f_audio_playback_work() -> u_audio_playback()
dereferences) and before kfree(audio). The endpoint is already disabled by
the time f_audio_disable() has run, so the work cannot be re-queued and
cancel_work_sync() is sufficient.

Found by static analysis and an automated reviewer; the playback_work
use-after-free was reproduced with KASAN under dummy_hcd (slab-use-after-
free in u_audio_playback() from f_audio_playback_work() after
f_audio_free(); a delay was added to the streaming path to make the
schedule-vs-free race deterministic). The out_ep/f_audio_complete
use-after-free is established by code inspection.

Fixes: c6994e6f067c ("USB: gadget: add USB Audio Gadget driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
v3: Fix the v2 regressions found in review:
    - Do NOT cancel_work_sync() in f_audio_disable(): that callback runs in
      atomic context (reset_config() under cdev->lock), so the sleeping
      cancel would be a sleep-in-atomic. Only usb_ep_disable() (atomic-safe)
      stays in f_audio_disable(); the work is drained from f_audio_free().
    - Drop the play_queue/copy_buf drain added in v2: audio->copy_buf can
      alias a buffer already on play_queue (f_audio_set_alt altset 0 queues
      it without clearing the pointer) and can hold ERR_PTR(-ENOMEM), so
      freeing it there double-freed / dereferenced an error pointer. That
      buffer-leak + copy_buf aliasing is a separate pre-existing issue and
      is left for a dedicated fix.
    - Correct the Fixes: tag to the original driver commit c6994e6f067c.

v2: https://lore.kernel.org/linux-usb/20261108122645.421721-1-hj351016@gmail.com/
 drivers/usb/gadget/function/f_uac1_legacy.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index 7c8d60c..c07f323 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -697,7 +697,17 @@ static int f_audio_get_alt(struct usb_function *f, unsigned intf)
 
 static void f_audio_disable(struct usb_function *f)
 {
-	return;
+	struct f_audio *audio = func_to_audio(f);
+
+	/*
+	 * Disable the OUT endpoint: the UDC gives back any outstanding
+	 * usb_requests, so f_audio_complete() stops dereferencing @audio and
+	 * no further playback_work is queued. This callback runs in atomic
+	 * context (reset_config() holds cdev->lock), so it must not sleep;
+	 * the work is drained from f_audio_free() instead.
+	 */
+	if (audio->out_ep)
+		usb_ep_disable(audio->out_ep);
 }
 
 /*-------------------------------------------------------------------------*/
@@ -943,6 +953,15 @@ static void f_audio_free(struct usb_function *f)
 	struct f_audio *audio = func_to_audio(f);
 	struct f_uac1_legacy_opts *opts;
 
+	/*
+	 * The OUT endpoint was disabled in f_audio_disable(), so no new
+	 * playback_work can be queued. Drain any still-pending work here, in
+	 * sleepable context and before gaudio_cleanup() tears down the card
+	 * that f_audio_playback_work() -> u_audio_playback() dereferences,
+	 * otherwise the work runs against freed memory.
+	 */
+	cancel_work_sync(&audio->playback_work);
+
 	gaudio_cleanup(&audio->card);
 	opts = container_of(f->fi, struct f_uac1_legacy_opts, func_inst);
 	kfree(audio);
-- 
2.43.0