[PATCH v2 2/3] HID: nintendo-switch2-usb: Improve shutdown logic with inbound message work

[Vicki Pfau <vi@endrift.com>]

When detaching the USB interface the driver marks the interface as shutdown
to prevent work that may be executing in parallel from attempting to use
the URBs before freeing the URBs. There is theoretically a very narrow
window in which the work could be scheduled while the shutdown is running
in a separate thread, starting to execute before the shutdown cancels the
work.

By ensuring the scheduling happens within the locked scope and adding an
additional shutdown check, this closes the window entirely.

Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
Link: https://lore.kernel.org/linux-input/a2dc00e7-9cab-4315-87a4-795f256c1e03@endrift.com/T/
Fixes: 9417132df63e ("HID: nintendo: Add preliminary Switch 2 controller driver")
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/hid/hid-nintendo-switch2-usb.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-nintendo-switch2-usb.c b/drivers/hid/hid-nintendo-switch2-usb.c
index 8b3b6841035d..f53cbf0a3e15 100644
--- a/drivers/hid/hid-nintendo-switch2-usb.c
+++ b/drivers/hid/hid-nintendo-switch2-usb.c
@@ -52,12 +52,10 @@ static void switch2_bulk_in(struct urb *urb)
 {
 	struct switch2_usb *ns2_usb = urb->context;
 	int i;
-	bool schedule = false;
 	unsigned long flags;
 
 	switch (urb->status) {
 	case 0:
-		schedule = true;
 		break;
 	case -ECONNRESET:
 	case -ENOENT:
@@ -71,16 +69,14 @@ static void switch2_bulk_in(struct urb *urb)
 	}
 
 	spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
-	if (ns2_usb->shutdown)
-		schedule = false;
-
 	for (i = 0; i < NS2_IN_URBS; i++) {
 		int err;
 		struct switch2_urb *ns2_urb;
 
 		if (ns2_usb->bulk_in[i].urb == urb) {
-			if (schedule) {
+			if (urb->status == 0 && !ns2_usb->shutdown) {
 				ns2_usb->bulk_in[i].state = NS2_URB_IN;
+				schedule_work(&ns2_usb->message_in_work);
 				continue;
 			} else {
 				ns2_usb->bulk_in[i].state = NS2_URB_FREE;
@@ -115,9 +111,6 @@ static void switch2_bulk_in(struct urb *urb)
 		}
 	}
 	spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
-
-	if (schedule)
-		schedule_work(&ns2_usb->message_in_work);
 }
 
 static void switch2_bulk_out(struct urb *urb)
@@ -295,6 +288,9 @@ static void switch2_usb_message_in_work(struct work_struct *work)
 
 	spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
 	for (i = 0; i < NS2_IN_URBS; i++) {
+		if (ns2_usb->shutdown)
+			break;
+
 		urb = &ns2_usb->bulk_in[i];
 		if (urb->state != NS2_URB_IN)
 			continue;
@@ -311,7 +307,7 @@ static void switch2_usb_message_in_work(struct work_struct *work)
 		}
 
 		spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
-		if (!urb->urb)
+		if (ns2_usb->shutdown || !urb->urb)
 			/* We're shutting down and this URB was freed */
 			break;
 
-- 
2.54.0