Forwarded: [PATCH] ath9k: defer reg_in URB resubmission to workqueue to fix RCU stall

[syzbot <syzbot+9b95da55ba5146a60734@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] ath9k: defer reg_in URB resubmission to workqueue to fix RCU stall
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master


ath9k_hif_usb_reg_in_cb() is a URB completion callback that
runs in softirq context via dummy_hcd's hrtimer which is
registered with HRTIMER_MODE_REL_SOFT.

Calling usb_submit_urb() directly from this softirq context
triggers a long synchronous chain:

  dummy_urb_enqueue()
    hrtimer_start(HRTIMER_MODE_REL_SOFT)
      dummy_timer()
        __usb_hcd_giveback_urb()
          ath9k_hif_usb_reg_in_cb()
            usb_submit_urb()  <- back to start

This keeps the CPU busy in softirq context indefinitely,
starving the rcu_preempt kthread and causing an RCU stall:

  rcu: rcu_preempt kthread starved for 3053 jiffies!
  rcu: Unless rcu_preempt kthread gets sufficient CPU time,
       OOM is now expected behavior.

Fix this by introducing a small per-resubmission wrapper
struct (reg_in_work) that is freshly allocated on each URB
completion and carries its own work_struct. The resubmission
is deferred to a dedicated ordered workqueue (reg_in_wq)
via queue_work(), allowing the softirq to exit quickly.

Using a fresh wrapper per completion avoids races that would
arise from reusing a single embedded work_struct:
  - INIT_WORK() is called on a newly allocated struct so
    there is no risk of reinitialising a work item that is
    still queued or running.
  - queue_work() on a fresh work_struct always succeeds so
    no resubmission is ever silently dropped.
  - usb_get_urb() is called before queue_work() and
    usb_put_urb() is called last in the worker after all
    URB accesses are complete, ensuring the URB remains
    valid for the entire lifetime of the work item.

A dedicated ordered workqueue is used instead of the system
workqueue to allow proper synchronization on disconnect.
destroy_workqueue() in ath9k_hif_usb_dealloc_reg_in_urbs()
drains all pending resubmissions before hif_dev is freed,
preventing use-after-free when the device is disconnected
while work items are still pending.

On resubmission failure in the worker the original error
path is preserved: skb and rx_buf are freed and
urb->context is set to NULL before dropping the URB
reference, matching the behaviour of the original
goto free_skb path.

Reported-by: syzbot+9b95da55ba5146a60734@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=9b95da55ba5146a60734
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 58 ++++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/hif_usb.h |  1 +
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 8533b88974b2..370764681749 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -731,12 +731,43 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	kfree(rx_buf);
 }
 
+struct reg_in_work {
+	struct urb		*urb;
+	struct hif_device_usb	*hif_dev;
+	struct work_struct	work;
+};
+
+static void ath9k_hif_usb_reg_in_resubmit(struct work_struct *work)
+{
+	struct reg_in_work *rw = container_of(work,
+					      struct reg_in_work,
+					      work);
+	struct urb *urb = rw->urb;
+	struct rx_buf *rx_buf = urb->context;
+
+	int ret;
+
+	usb_anchor_urb(rw->urb, &rw->hif_dev->reg_in_submitted);
+	ret = usb_submit_urb(rw->urb, GFP_KERNEL);
+
+	if (ret) {
+		usb_unanchor_urb(rw->urb);
+		if (rx_buf) {
+			kfree_skb(rx_buf->skb);
+			kfree(rx_buf);
+			urb->context = NULL;
+		}
+	}
+
+	usb_put_urb(urb);
+	kfree(rw);
+}
+
 static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 {
 	struct rx_buf *rx_buf = urb->context;
 	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
 	struct sk_buff *skb = rx_buf->skb;
-	int ret;
 
 	if (!skb)
 		return;
@@ -786,14 +817,20 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 	}
 
 resubmit:
-	usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
-	ret = usb_submit_urb(urb, GFP_ATOMIC);
-	if (ret) {
-		usb_unanchor_urb(urb);
-		goto free_skb;
+	{
+		struct reg_in_work *rw;
+
+		rw = kmalloc_obj(*rw, GFP_ATOMIC);
+		if (!rw)
+			goto free_skb;
+
+		rw->urb = urb;
+		rw->hif_dev = hif_dev;
+		usb_get_urb(urb);
+		INIT_WORK(&rw->work, ath9k_hif_usb_reg_in_resubmit);
+		queue_work(hif_dev->reg_in_wq, &rw->work);
+		return;
 	}
-
-	return;
 free_skb:
 	kfree_skb(skb);
 free_rx_buf:
@@ -959,6 +996,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 static void ath9k_hif_usb_dealloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 {
 	usb_kill_anchored_urbs(&hif_dev->reg_in_submitted);
+	if (hif_dev->reg_in_wq)
+		destroy_workqueue(hif_dev->reg_in_wq);
 }
 
 static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
@@ -969,6 +1008,9 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->reg_in_submitted);
+	hif_dev->reg_in_wq = alloc_ordered_workqueue("ath9k_reg_in", 0);
+	if (!hif_dev->reg_in_wq)
+		return -ENOMEM;
 
 	for (i = 0; i < MAX_REG_IN_URB_NUM; i++) {
 
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index b3e66b0485a5..38f17a12fd5f 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -124,6 +124,7 @@ struct hif_device_usb {
 	struct usb_anchor regout_submitted;
 	struct usb_anchor rx_submitted;
 	struct usb_anchor reg_in_submitted;
+	struct workqueue_struct *reg_in_wq;
 	struct usb_anchor mgmt_submitted;
 	struct sk_buff *remain_skb;
 	char fw_name[64];
-- 
2.43.0