usbnet: fix cyclical race on disconnect with work queue

The work can submit URBs and the URBs can schedule the work.
This cycle needs to be broken, when a device is to be stopped.
Use a flag to do so.
This is a design issue as old as the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
CC: stable@vger.kernel.org
Link: https://patch.msgid.link/20240919123525.688065-1-oneukum@suse.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 18eb5ba436df69027bff6e34bb13bf302ad2da4c..2506aa8c603ec0f965bb8ebe5439e7596a47e564 100644 (file)
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -464,10 +464,15 @@ static enum skb_state defer_bh(struct usbnet *dev, struct sk_buff *skb,
 void usbnet_defer_kevent (struct usbnet *dev, int work)
 {
        set_bit (work, &dev->flags);
-       if (!schedule_work (&dev->kevent))
-               netdev_dbg(dev->net, "kevent %s may have been dropped\n", usbnet_event_names[work]);
-       else
-               netdev_dbg(dev->net, "kevent %s scheduled\n", usbnet_event_names[work]);
+       if (!usbnet_going_away(dev)) {
+               if (!schedule_work(&dev->kevent))
+                       netdev_dbg(dev->net,
+                                  "kevent %s may have been dropped\n",
+                                  usbnet_event_names[work]);
+               else
+                       netdev_dbg(dev->net,
+                                  "kevent %s scheduled\n", usbnet_event_names[work]);
+       }
 }
 EXPORT_SYMBOL_GPL(usbnet_defer_kevent);
 
@@ -535,7 +540,8 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags)
                        tasklet_schedule (&dev->bh);
                        break;
                case 0:
-                       __usbnet_queue_skb(&dev->rxq, skb, rx_start);
+                       if (!usbnet_going_away(dev))
+                               __usbnet_queue_skb(&dev->rxq, skb, rx_start);
                }
        } else {
                netif_dbg(dev, ifdown, dev->net, "rx: stopped\n");
@@ -843,9 +849,18 @@ int usbnet_stop (struct net_device *net)
 
        /* deferred work (timer, softirq, task) must also stop */
        dev->flags = 0;
-       del_timer_sync (&dev->delay);
-       tasklet_kill (&dev->bh);
+       del_timer_sync(&dev->delay);
+       tasklet_kill(&dev->bh);
        cancel_work_sync(&dev->kevent);
+
+       /* We have cyclic dependencies. Those calls are needed
+        * to break a cycle. We cannot fall into the gaps because
+        * we have a flag
+        */
+       tasklet_kill(&dev->bh);
+       del_timer_sync(&dev->delay);
+       cancel_work_sync(&dev->kevent);
+
        if (!pm)
                usb_autopm_put_interface(dev->intf);
 
@@ -1171,7 +1186,8 @@ fail_halt:
                                           status);
                } else {
                        clear_bit (EVENT_RX_HALT, &dev->flags);
-                       tasklet_schedule (&dev->bh);
+                       if (!usbnet_going_away(dev))
+                               tasklet_schedule(&dev->bh);
                }
        }
 
@@ -1196,7 +1212,8 @@ fail_halt:
                        usb_autopm_put_interface(dev->intf);
 fail_lowmem:
                        if (resched)
-                               tasklet_schedule (&dev->bh);
+                               if (!usbnet_going_away(dev))
+                                       tasklet_schedule(&dev->bh);
                }
        }
 
@@ -1559,6 +1576,7 @@ static void usbnet_bh (struct timer_list *t)
        } else if (netif_running (dev->net) &&
                   netif_device_present (dev->net) &&
                   netif_carrier_ok(dev->net) &&
+                  !usbnet_going_away(dev) &&
                   !timer_pending(&dev->delay) &&
                   !test_bit(EVENT_RX_PAUSED, &dev->flags) &&
                   !test_bit(EVENT_RX_HALT, &dev->flags)) {
@@ -1606,6 +1624,7 @@ void usbnet_disconnect (struct usb_interface *intf)
        usb_set_intfdata(intf, NULL);
        if (!dev)
                return;
+       usbnet_mark_going_away(dev);
 
        xdev = interface_to_usbdev (intf);
 

diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9f08a584d707853c442a018329c156dcef76de6e..0b9f1e598e3a6bb0285e35918a361d74cd1dd71f 100644 (file)
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -76,8 +76,23 @@ struct usbnet {
 #              define EVENT_LINK_CHANGE        11
 #              define EVENT_SET_RX_MODE        12
 #              define EVENT_NO_IP_ALIGN        13
+/* This one is special, as it indicates that the device is going away
+ * there are cyclic dependencies between tasklet, timer and bh
+ * that must be broken
+ */
+#              define EVENT_UNPLUG             31
 };
 
+static inline bool usbnet_going_away(struct usbnet *ubn)
+{
+       return test_bit(EVENT_UNPLUG, &ubn->flags);
+}
+
+static inline void usbnet_mark_going_away(struct usbnet *ubn)
+{
+       set_bit(EVENT_UNPLUG, &ubn->flags);
+}
+
 static inline struct usb_driver *driver_of(struct usb_interface *intf)
 {
        return to_usb_driver(intf->dev.driver);