| From 04e906839a053f092ef53f4fb2d610983412b904 Mon Sep 17 00:00:00 2001 |
| From: Oliver Neukum <oneukum@suse.com> |
| Date: Thu, 19 Sep 2024 14:33:42 +0200 |
| Subject: usbnet: fix cyclical race on disconnect with work queue |
| |
| From: Oliver Neukum <oneukum@suse.com> |
| |
| commit 04e906839a053f092ef53f4fb2d610983412b904 upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/net/usb/usbnet.c | 37 ++++++++++++++++++++++++++++--------- |
| include/linux/usb/usbnet.h | 15 +++++++++++++++ |
| 2 files changed, 43 insertions(+), 9 deletions(-) |
| |
| --- a/drivers/net/usb/usbnet.c |
| +++ b/drivers/net/usb/usbnet.c |
| @@ -464,10 +464,15 @@ static enum skb_state defer_bh(struct us |
| 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 |
| 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 |
| } 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_inter |
| usb_set_intfdata(intf, NULL); |
| if (!dev) |
| return; |
| + usbnet_mark_going_away(dev); |
| |
| xdev = interface_to_usbdev (intf); |
| |
| --- 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); |