| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240321124758.6302-1-oneukum@suse.com>
Date: Thu, 21 Mar 2024 13:46:41 +0100
From: Oliver Neukum <oneukum@...e.com>
To: davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com,
netdev@...r.kernel.org,
linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: Oliver Neukum <oneukum@...e.com>,
syzbot+9665bf55b1c828bbcd8a@...kaller.appspotmail.com
Subject: [PATCH net-next] 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.
Fixes: f29fc259976e9 ("[PATCH] USB: usbnet (1/9) clean up framing")
Reported-by: syzbot+9665bf55b1c828bbcd8a@...kaller.appspotmail.com
Signed-off-by: Oliver Neukum <oneukum@...e.com>
---
drivers/net/usb/usbnet.c | 37 ++++++++++++++++++++++++++++---------
include/linux/usb/usbnet.h | 18 ++++++++++++++++++
2 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e84efa661589..422d91635045 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -467,10 +467,12 @@ 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);
@@ -538,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");
@@ -849,6 +852,16 @@ int usbnet_stop (struct net_device *net)
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);
@@ -1174,7 +1187,8 @@ usbnet_deferred_kevent (struct work_struct *work)
status);
} else {
clear_bit (EVENT_RX_HALT, &dev->flags);
- tasklet_schedule (&dev->bh);
+ if (!usbnet_going_away(dev))
+ tasklet_schedule (&dev->bh);
}
}
@@ -1196,10 +1210,13 @@ usbnet_deferred_kevent (struct work_struct *work)
}
if (rx_submit (dev, urb, GFP_KERNEL) == -ENOLINK)
resched = 0;
- usb_autopm_put_interface(dev->intf);
fail_lowmem:
- if (resched)
+ usb_autopm_put_interface(dev->intf);
+ if (resched) {
+ set_bit (EVENT_RX_MEMORY, &dev->flags);
+
tasklet_schedule (&dev->bh);
+ }
}
}
@@ -1212,13 +1229,13 @@ usbnet_deferred_kevent (struct work_struct *work)
if (status < 0)
goto skip_reset;
if(info->link_reset && (retval = info->link_reset(dev)) < 0) {
- usb_autopm_put_interface(dev->intf);
skip_reset:
netdev_info(dev->net, "link reset failed (%d) usbnet usb-%s-%s, %s\n",
retval,
dev->udev->bus->bus_name,
dev->udev->devpath,
info->description);
+ usb_autopm_put_interface(dev->intf);
} else {
usb_autopm_put_interface(dev->intf);
}
@@ -1562,6 +1579,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)) {
@@ -1609,6 +1627,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 9f08a584d707..d26599faab33 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -76,8 +76,26 @@ 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)
+{
+ smp_mb__before_atomic();
+ return test_bit(EVENT_UNPLUG, &ubn->flags);
+}
+
+static inline void usbnet_mark_going_away(struct usbnet *ubn)
+{
+ set_bit(EVENT_UNPLUG, &ubn->flags);
+ smp_mb__after_atomic();
+}
+
static inline struct usb_driver *driver_of(struct usb_interface *intf)
{
return to_usb_driver(intf->dev.driver);
--
2.44.0
Powered by blists - more mailing lists