mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 14:33:24 -04:00
wifi: rtl8xxxu: fix use-after-free from rx_urb_wq on stop
rtl8xxxu arms rx_urb_wq from the RX completion path:
rtl8xxxu_rx_complete() hands the URB to rtl8xxxu_queue_rx_urb(), which
queues it on rx_urb_pending_list and, once the list grows past
RTL8XXXU_RX_URB_PENDING_WATER, schedules rx_urb_wq. The worker
rtl8xxxu_rx_urb_work() drains rx_urb_pending_list, recovers priv through
container_of, and resubmits each URB through rtl8xxxu_submit_rx_urb(),
which anchors it on rx_anchor and dereferences priv->udev.
rtl8xxxu_stop() cancels the sibling work items (c2hcmd_work, ra_watchdog,
update_beacon_work) but never cancels rx_urb_wq, so a worker armed during
the last burst of RX traffic can run rtl8xxxu_rx_urb_work() after
rtl8xxxu_disconnect() has called ieee80211_free_hw(), which frees priv,
producing a use-after-free. The window opens under active RX traffic
(pending count above the watermark) followed by a disconnect.
There are two teardown races to close:
* rtl8xxxu_queue_rx_urb() decided whether to enqueue under rx_urb_lock
but called schedule_work() after dropping the lock. A completion
that observed shutdown == false and released the lock could then call
schedule_work() after rtl8xxxu_stop() had set shutdown and
cancel_work_sync() had already returned, arming the worker to run
after the teardown. Move schedule_work() under the same !shutdown
branch so the arming decision is atomic with the shutdown check.
* rtl8xxxu_rx_urb_work() anchors every URB it drained back onto
rx_anchor through rtl8xxxu_submit_rx_urb(). A worker still running
when usb_kill_anchored_urbs(&priv->rx_anchor) returned would submit a
URB that escaped the kill. In rtl8xxxu_stop(), call
cancel_work_sync(&priv->rx_urb_wq) before the kill so the worker is
drained first.
After priv->shutdown is set under rx_urb_lock, completions can no longer
queue rx_urb_wq. cancel_work_sync() then drains the last queued or running
worker, and the following usb_kill_anchored_urbs() kills the URBs it may
have submitted.
rtl8xxxu_disconnect() is covered because ieee80211_unregister_hw()
guarantees .stop() runs for a live interface before ieee80211_free_hw()
frees priv. The probe error path needs no cancel: rx_urb_wq is
INIT_WORK()'d there but cannot have been scheduled, since no URB is
submitted before ieee80211_register_hw() succeeds.
This bug was found by static analysis.
Fixes: 26f1fad29a ("New driver: rtl8xxxu (mac80211)")
Cc: stable@vger.kernel.org
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Link: https://patch.msgid.link/20260630033117.3377-1-fanwu01@zju.edu.cn
This commit is contained in:
@@ -5837,14 +5837,19 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
|
||||
{
|
||||
struct sk_buff *skb;
|
||||
unsigned long flags;
|
||||
int pending = 0;
|
||||
|
||||
spin_lock_irqsave(&priv->rx_urb_lock, flags);
|
||||
|
||||
if (!priv->shutdown) {
|
||||
list_add_tail(&rx_urb->list, &priv->rx_urb_pending_list);
|
||||
priv->rx_urb_pending_count++;
|
||||
pending = priv->rx_urb_pending_count;
|
||||
/*
|
||||
* Arm the worker under rx_urb_lock so this is atomic with the
|
||||
* shutdown check: moving it out of the lock would let a
|
||||
* completion arm the work after rtl8xxxu_stop() canceled it.
|
||||
*/
|
||||
if (priv->rx_urb_pending_count > RTL8XXXU_RX_URB_PENDING_WATER)
|
||||
schedule_work(&priv->rx_urb_wq);
|
||||
} else {
|
||||
skb = (struct sk_buff *)rx_urb->urb.context;
|
||||
dev_kfree_skb_irq(skb);
|
||||
@@ -5852,9 +5857,6 @@ static void rtl8xxxu_queue_rx_urb(struct rtl8xxxu_priv *priv,
|
||||
}
|
||||
|
||||
spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
|
||||
|
||||
if (pending > RTL8XXXU_RX_URB_PENDING_WATER)
|
||||
schedule_work(&priv->rx_urb_wq);
|
||||
}
|
||||
|
||||
static void rtl8xxxu_rx_urb_work(struct work_struct *work)
|
||||
@@ -7506,6 +7508,13 @@ static void rtl8xxxu_stop(struct ieee80211_hw *hw, bool suspend)
|
||||
priv->shutdown = true;
|
||||
spin_unlock_irqrestore(&priv->rx_urb_lock, flags);
|
||||
|
||||
/*
|
||||
* Cancel before killing rx_anchor: the worker re-anchors every URB
|
||||
* it drained via rtl8xxxu_submit_rx_urb(), so a worker still running
|
||||
* after the kill could submit a URB that escapes it.
|
||||
*/
|
||||
cancel_work_sync(&priv->rx_urb_wq);
|
||||
|
||||
usb_kill_anchored_urbs(&priv->rx_anchor);
|
||||
usb_kill_anchored_urbs(&priv->tx_anchor);
|
||||
if (priv->usb_interrupts)
|
||||
|
||||
Reference in New Issue
Block a user