All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: usb-audio: fix list_add double-add in push_back_to_ready_list
@ 2026-09-15 16:31 Nguyen Ngoc Thang
  2026-09-16  7:31 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-15 16:31 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai
  Cc: linux-sound, syzbot+9fe3b8d9f5c64ff410a7, Nguyen Ngoc Thang

stop_urbs() clears ep->ready_playback_urbs with a bare INIT_LIST_HEAD()
instead of unlinking each queued snd_urb_ctx. If a URB survives past
wait_clear_urbs()'s forced STOPPING->STOPPED timeout, its ctx is left
looking "linked" (stale next/prev) even though the list head has
forgotten it. When the endpoint later restarts and re-queues that same
ctx onto the (now real) ready list, and the old URB's completion
handler then calls push_back_to_ready_list() for it a second time, the
ctx is still the list's own tail and list_add's double-add check trips:

  kernel BUG at lib/list_debug.c:35 (list_add double add)

Guard push_back_to_ready_list() with a list_empty() check so a
still-linked ctx isn't re-added, and make stop_urbs() actually unlink
each ctx via list_del_init() instead of only resetting the head, so a
dropped ctx doesn't keep looking linked to that guard.

Reported-by: syzbot+9fe3b8d9f5c64ff410a7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9fe3b8d9f5c64ff410a7
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Note: I was not able to trigger the actual crash. dummy_hcd doesn't
expose isochronous endpoints by default; after patching that locally
for testing, 1600 PREPARE/START/DRAIN cycles plus a
WARN_ON(!list_empty(&ctx->ready_list)) probe in push_back_to_ready_list()
never hit the race window (wait_clear_urbs() never logged a "timeout:
still N active urbs", so the forced STOPPING->STOPPED path was never
even exercised in this setup). This fix is based on static analysis of
the reported stack trace and the stop_urbs()/wait_clear_urbs()/
push_back_to_ready_list() interaction, not a confirmed repro. Sharing
in case someone with real hardware or a working reproducer can verify.

 sound/usb/endpoint.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 0835943d7b0e..ddbd8ab22b50 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -449,7 +449,9 @@ static void push_back_to_ready_list(struct snd_usb_endpoint *ep,
 				    struct snd_urb_ctx *ctx)
 {
 	guard(spinlock_irqsave)(&ep->lock);
-	list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs);
+	/* ctx may still be linked: a stale completion racing a stop/restart. */
+	if (list_empty(&ctx->ready_list))
+		list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs);
 }
 
 /*
@@ -1036,6 +1038,7 @@ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep)
  */
 static int stop_urbs(struct snd_usb_endpoint *ep, bool force, bool keep_pending)
 {
+	struct snd_urb_ctx *ctx, *n;
 	unsigned int i;
 
 	if (!force && atomic_read(&ep->running))
@@ -1045,7 +1048,9 @@ static int stop_urbs(struct snd_usb_endpoint *ep, bool force, bool keep_pending)
 		return 0;
 
 	scoped_guard(spinlock_irqsave, &ep->lock) {
-		INIT_LIST_HEAD(&ep->ready_playback_urbs);
+		/* Unlink each ctx; INIT_LIST_HEAD() alone would leave them looking linked. */
+		list_for_each_entry_safe(ctx, n, &ep->ready_playback_urbs, ready_list)
+			list_del_init(&ctx->ready_list);
 		ep->next_packet_head = 0;
 		ep->next_packet_queued = 0;
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ALSA: usb-audio: fix list_add double-add in push_back_to_ready_list
  2026-09-15 16:31 [PATCH] ALSA: usb-audio: fix list_add double-add in push_back_to_ready_list Nguyen Ngoc Thang
@ 2026-09-16  7:31 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-09-16  7:31 UTC (permalink / raw)
  To: Nguyen Ngoc Thang
  Cc: Jaroslav Kysela, Takashi Iwai, linux-sound,
	syzbot+9fe3b8d9f5c64ff410a7

On Tue, 15 Sep 2026 18:31:10 +0200,
Nguyen Ngoc Thang wrote:
> 
> stop_urbs() clears ep->ready_playback_urbs with a bare INIT_LIST_HEAD()
> instead of unlinking each queued snd_urb_ctx. If a URB survives past
> wait_clear_urbs()'s forced STOPPING->STOPPED timeout, its ctx is left
> looking "linked" (stale next/prev) even though the list head has
> forgotten it. When the endpoint later restarts and re-queues that same
> ctx onto the (now real) ready list, and the old URB's completion
> handler then calls push_back_to_ready_list() for it a second time, the
> ctx is still the list's own tail and list_add's double-add check trips:
> 
>   kernel BUG at lib/list_debug.c:35 (list_add double add)
> 
> Guard push_back_to_ready_list() with a list_empty() check so a
> still-linked ctx isn't re-added, and make stop_urbs() actually unlink
> each ctx via list_del_init() instead of only resetting the head, so a
> dropped ctx doesn't keep looking linked to that guard.
> 
> Reported-by: syzbot+9fe3b8d9f5c64ff410a7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9fe3b8d9f5c64ff410a7
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> Note: I was not able to trigger the actual crash. dummy_hcd doesn't
> expose isochronous endpoints by default; after patching that locally
> for testing, 1600 PREPARE/START/DRAIN cycles plus a
> WARN_ON(!list_empty(&ctx->ready_list)) probe in push_back_to_ready_list()
> never hit the race window (wait_clear_urbs() never logged a "timeout:
> still N active urbs", so the forced STOPPING->STOPPED path was never
> even exercised in this setup). This fix is based on static analysis of
> the reported stack trace and the stop_urbs()/wait_clear_urbs()/
> push_back_to_ready_list() interaction, not a confirmed repro. Sharing
> in case someone with real hardware or a working reproducer can verify.

Applied now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-16  7:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 16:31 [PATCH] ALSA: usb-audio: fix list_add double-add in push_back_to_ready_list Nguyen Ngoc Thang
2026-09-16  7:31 ` Takashi Iwai

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.