[PATCH v2] usb: gadget: u_audio: call period elapsed under the stream lock

[PATCH v2] usb: gadget: u_audio: call period elapsed under the stream lock

[Yaohan Li]

u_audio_iso_complete() drops the substream stream lock before
calling snd_pcm_period_elapsed(), which re-acquires it internally.
Between the unlock and the re-acquire, a concurrent close can run
through snd_pcm_drop(), finish the PCM release and free the runtime,
leaving the period notification to dereference a freed runtime
(runtime->fasync in snd_pcm_period_elapsed()).

Keep the stream lock across the period notification and use
snd_pcm_period_elapsed_under_stream_lock(), the API for exactly this
caller-held-lock case, so the callback's runtime access happens
inside a single locked section.

On final PCM release, snd_pcm_drop() takes the stream lock before
runtime detachment: an in-flight completion holding that lock
finishes before STOP can proceed.  The STOP trigger clears prm->ss,
so subsequent callbacks do not enter the running PCM path.  Runtime
detachment happens later and is not itself protected by the stream
lock.  On the gadget side, u_audio_stop_*() dequeues the requests
before the endpoint is freed.

KASAN reported the use-after-free in u_audio_iso_complete() on the
unmodified kernel.  With the callback stalled inside the locked
section while a concurrent close ran, no use-after-free was reported
after this change.

This issue was investigated and the fix was developed with AI
assistance (ZCode/GLM-4).

Fixes: 773e53d50e227 ("usb: gadget: u_audio: remove cached period bytes value")
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---

Changes since v1:
- v1 saved the period size while the lock was held but still called
  snd_pcm_period_elapsed() after dropping it, leaving the same
  unlock/re-lock window open for the period notification itself.
  This version keeps the stream lock across the notification using
  snd_pcm_period_elapsed_under_stream_lock().
- Documented the synchronization this relies on: snd_pcm_drop()
  takes the stream lock before runtime detachment, so an in-flight
  completion holding the lock finishes first, and its STOP trigger
  clears u_audio's prm->ss so later callbacks take the early exit.
  Runtime detachment itself is not protected by the stream lock.
v1: https://lore.kernel.org/r/179378417544.69464.16164899430939223277@gmail.com/

 drivers/usb/gadget/function/u_audio.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index f0ee83abcebc7..7da4032e3a203 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -156,6 +156,7 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 	struct uac_rtd_params *prm = req->context;
 	struct snd_uac_chip *uac = prm->uac;
 	unsigned int frames, p_pktsize;
+	unsigned int period_bytes;
 	unsigned long long pitched_rate_mil, p_pktsize_residue_mil,
 			residue_frames_mil, div_result;
 
@@ -267,10 +268,12 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
 	/* update hw_ptr after data is copied to memory */
 	prm->hw_ptr = (hw_ptr + req->actual) % runtime->dma_bytes;
 	hw_ptr = prm->hw_ptr;
-	snd_pcm_stream_unlock(substream);
+	period_bytes = snd_pcm_lib_period_bytes(substream);
+
+	if ((hw_ptr % period_bytes) < req->actual)
+		snd_pcm_period_elapsed_under_stream_lock(substream);
 
-	if ((hw_ptr % snd_pcm_lib_period_bytes(substream)) < req->actual)
-		snd_pcm_period_elapsed(substream);
+	snd_pcm_stream_unlock(substream);
 
 exit:
 	if (usb_ep_queue(ep, req, GFP_ATOMIC))

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.34.1

Re: [PATCH v2] usb: gadget: u_audio: call period elapsed under the stream lock

[sashiko-bot]

> u_audio_iso_complete() drops the substream stream lock before
> calling snd_pcm_period_elapsed(), which re-acquires it internally.
> Between the unlock and the re-acquire, a concurrent close can run
> through snd_pcm_drop(), finish the PCM release and free the runtime,
> leaving the period notification to dereference a freed runtime
> (runtime->fasync in snd_pcm_period_elapsed()).
> 
> Keep the stream lock across the period notification and use
> snd_pcm_period_elapsed_under_stream_lock(), the API for exactly this
> caller-held-lock case, so the callback's runtime access happens
> inside a single locked section.
> 
> On final PCM release, snd_pcm_drop() takes the stream lock before
> runtime detachment: an in-flight completion holding that lock
> finishes before STOP can proceed.  The STOP trigger clears prm->ss,
> so subsequent callbacks do not enter the running PCM path.  Runtime
> detachment happens later and is not itself protected by the stream
> lock.  On the gadget side, u_audio_stop_*() dequeues the requests
> before the endpoint is freed.
> 
> KASAN reported the use-after-free in u_audio_iso_complete() on the
> unmodified kernel.  With the callback stalled inside the locked
> section while a concurrent close ran, no use-after-free was reported
> after this change.
> 
> This issue was investigated and the fix was developed with AI
> assistance (ZCode/GLM-4).
> 
> Fixes: 773e53d50e227 ("usb: gadget: u_audio: remove cached period bytes value")
> Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>

Sashiko has reviewed this patch and found no issues.

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