[PATCH 1/3] usb: core: Fix use-after-free in anchor wakeup protocol
From: Hongyu Xie
Date: Wed Oct 21 2026 - 23:56:24 EST
The API contract of usb_wait_anchor_empty_timeout() and of the anchor
cleanup helpers is that when they return successfully (non-zero, for
usb_wait_anchor_empty_timeout()), no URB giveback is in flight for the
anchor anymore, so drivers may free the memory holding the anchor
(xpad, for example, embeds irq_out_anchor in its private structure and
kfrees it in xpad_disconnect()).
This contract is broken by two windows in which the waiter can observe
"anchor list empty and suspend_wakeups == 0" while the URB giveback
path is still about to access anchor->wait:
1. usb_anchor_resume_wakeups() decrements suspend_wakeups, which
makes the wait condition true, and only afterwards calls
wake_up(&anchor->wait), without any synchronization in between.
A waiter returns, the driver frees the anchor, and the late
wake_up() then touches the freed wait queue head.
2. __usb_unanchor_urb() wakes up waiters while holding anchor->lock,
but usb_wait_anchor_empty_timeout() evaluated the same condition
without the lock, so it could observe the emptied urb_list in the
middle of that critical section and return before the wake_up()
(and the spin_unlock()) had run. Reading the two condition fields
locklessly could likewise observe a stale suspend_wakeups value
after the giveback's unanchor on weakly ordered systems.
Complete the locking protocol that commit fbc299437c06 ("usb: core:
Solve race condition in anchor cleanup functions") established for the
kill/poison/scuttle helpers: usb_wait_anchor_empty_timeout() now
evaluates the condition under anchor->lock (a small self-locking
wrapper around the lockless check), and usb_anchor_resume_wakeups()
performs the suspend_wakeups decrement together with the wakeup check
and the wake_up() within a single anchor->lock critical section.
With the decrement and the wake_up() atomic with respect to the locked
condition check, a waiter can only observe the condition true after
every giveback that has incremented suspend_wakeups has finished
accessing the anchor, so the guarantee that the wakeup is delayed
until the completion handler has run also extends to the anchor memory
itself.
The race has been reported by syzbot repeatedly since 2019, most
recently as:
BUG: KASAN: slab-use-after-free in register_lock_class+0x8d1/0x980 kernel/locking/lockdep.c:1333
[...]
__wake_up_common_lock+0x25/0x1e0 kernel/sched/wait.c:105
__usb_hcd_giveback_urb+0x3ac/0x530 drivers/usb/core/hcd.c:1653
Freed by task 7828:
[...]
kfree+0x14a/0x380 mm/slub.c:4390
xpad_disconnect+0x359/0x490 drivers/input/joystick/xpad.c:2194
and:
BUG: KASAN: use-after-free in register_lock_class+0xe6a/0x1120 kernel/locking/lockdep.c:1336
[...]
__wake_up_common_lock+0xb4/0x130 kernel/sched/wait.c:136
usb_anchor_resume_wakeups drivers/usb/core/urb.c:958 [inline]
usb_anchor_resume_wakeups+0xbe/0xe0 drivers/usb/core/urb.c:951
__usb_hcd_giveback_urb+0x2df/0x5c0 drivers/usb/core/hcd.c:1674
Freed by task 3889:
[...]
kfree+0x173/0x390 mm/slab.c:3786
xpad_disconnect+0x1cb/0x530 drivers/input/joystick/xpad.c:1905
Both reports lack a reproducer; the analysis was verified by walking
the two stacks against the code, not at runtime. All in-tree callers
of usb_wait_anchor_empty_timeout() (and every driver which frees an
anchor after the anchor cleanup helpers) are equally affected; xpad
is merely the one syzbot found.
Fixes: 6ec4147e7bdb ("usb-anchor: Delay usb_wait_anchor_empty_timeout wake up till completion is done")
Fixes: 51a2f077c44e ("USB: introduce usb_anchor")
Reported-by: syzbot+7f5e98b052c766f47f68@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://lore.kernel.org/r/00000000000090170a0614431f47@xxxxxxxxxx
Reported-by: syzbot+59d3714b44d79d02f944@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://lore.kernel.org/r/00000000000038435e05e9f94cd4@xxxxxxxxxx
Cc: stable@xxxxxxxxxxxxxxx
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Oliver Neukum <oliver@xxxxxxxxxx>
Cc: Hans de Goede <hansg@xxxxxxxxxx>
Signed-off-by: Hongyu Xie <xiehongyu1@xxxxxxxxxx>
---
drivers/usb/core/urb.c | 51 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
index c06b44ca507b..3afd184c9b32 100644
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -139,19 +139,39 @@ void usb_anchor_urb(struct urb *urb, struct usb_anchor *anchor)
}
EXPORT_SYMBOL_GPL(usb_anchor_urb);
-static int usb_anchor_check_wakeup(struct usb_anchor *anchor)
+/* Callers must hold anchor->lock */
+static int __usb_anchor_check_wakeup(struct usb_anchor *anchor)
{
return atomic_read(&anchor->suspend_wakeups) == 0 &&
list_empty(&anchor->urb_list);
}
+/*
+ * Same check as __usb_anchor_check_wakeup(), but takes anchor->lock. It is
+ * meant for waiters which do not hold the lock: the critical section
+ * serializes with the URB giveback path, so once the check succeeds no
+ * giveback that has incremented suspend_wakeups will touch the anchor
+ * anymore and the caller may free the memory holding the anchor.
+ */
+static int usb_anchor_check_wakeup(struct usb_anchor *anchor)
+{
+ unsigned long flags;
+ int wakeup;
+
+ spin_lock_irqsave(&anchor->lock, flags);
+ wakeup = __usb_anchor_check_wakeup(anchor);
+ spin_unlock_irqrestore(&anchor->lock, flags);
+
+ return wakeup;
+}
+
/* Callers must hold anchor->lock */
static void __usb_unanchor_urb(struct urb *urb, struct usb_anchor *anchor)
{
urb->anchor = NULL;
list_del(&urb->anchor_list);
usb_put_urb(urb);
- if (usb_anchor_check_wakeup(anchor))
+ if (__usb_anchor_check_wakeup(anchor))
wake_up(&anchor->wait);
}
@@ -827,7 +847,7 @@ void usb_kill_anchored_urbs(struct usb_anchor *anchor)
usb_put_urb(victim);
spin_lock_irq(&anchor->lock);
}
- surely_empty = usb_anchor_check_wakeup(anchor);
+ surely_empty = __usb_anchor_check_wakeup(anchor);
spin_unlock_irq(&anchor->lock);
cpu_relax();
@@ -866,7 +886,7 @@ void usb_poison_anchored_urbs(struct usb_anchor *anchor)
usb_put_urb(victim);
spin_lock_irq(&anchor->lock);
}
- surely_empty = usb_anchor_check_wakeup(anchor);
+ surely_empty = __usb_anchor_check_wakeup(anchor);
spin_unlock_irq(&anchor->lock);
cpu_relax();
@@ -919,12 +939,23 @@ EXPORT_SYMBOL_GPL(usb_anchor_suspend_wakeups);
*/
void usb_anchor_resume_wakeups(struct usb_anchor *anchor)
{
+ unsigned long flags;
+
if (!anchor)
return;
+ /*
+ * The decrement, the wakeup check and the wake_up() must happen
+ * under anchor->lock: usb_wait_anchor_empty_timeout() waiters
+ * evaluate the same condition under the lock and may free the
+ * anchor once they observe it true, so this function must not
+ * touch anchor->wait after that.
+ */
+ spin_lock_irqsave(&anchor->lock, flags);
atomic_dec(&anchor->suspend_wakeups);
- if (usb_anchor_check_wakeup(anchor))
+ if (__usb_anchor_check_wakeup(anchor))
wake_up(&anchor->wait);
+ spin_unlock_irqrestore(&anchor->lock, flags);
}
EXPORT_SYMBOL_GPL(usb_anchor_resume_wakeups);
@@ -941,6 +972,14 @@ EXPORT_SYMBOL_GPL(usb_anchor_resume_wakeups);
int usb_wait_anchor_empty_timeout(struct usb_anchor *anchor,
unsigned int timeout)
{
+ /*
+ * The condition must be evaluated under anchor->lock: when this
+ * function returns non-zero, every URB giveback that has incremented
+ * suspend_wakeups has finished with the anchor, so the caller may
+ * free the anchor, which is typically embedded in the driver's
+ * private structure. A zero return (timeout) means a giveback may
+ * still be in flight and gives no such guarantee.
+ */
return wait_event_timeout(anchor->wait,
usb_anchor_check_wakeup(anchor),
msecs_to_jiffies(timeout));
@@ -997,7 +1036,7 @@ void usb_scuttle_anchored_urbs(struct usb_anchor *anchor)
struct urb, anchor_list);
__usb_unanchor_urb(victim, anchor);
}
- surely_empty = usb_anchor_check_wakeup(anchor);
+ surely_empty = __usb_anchor_check_wakeup(anchor);
spin_unlock_irqrestore(&anchor->lock, flags);
cpu_relax();
--
2.32.0