diff mbox series

[net-next,5/9] lan78xx: fix race condition in PHY handling causing kernel lock up

Message ID 20210204113121.29786-6-john.efstathiades@pebblebay.com
State New
Headers show
Series [net-next,1/9] lan78xx: add NAPI interface support | expand

Commit Message

John Efstathiades Feb. 4, 2021, 11:31 a.m. UTC
If there is a device disconnect at roughly the same time as a deferred
PHY link reset there is a race condition that can result in kernel
lock up due to a null pointer dereference in the driver's deferred
work handling routine lan78xx_delayedwork().

Stop processing of deferred work items when the driver's USB disconnect
handler is invoked.

Disconnect the PHY only after the network device has been unregistered
and all delayed work has been been cancelled.

Signed-off-by: John Efstathiades <john.efstathiades@pebblebay.com>
---
 drivers/net/usb/lan78xx.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d2fcc3c5eff2..7c815061e02e 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -391,6 +391,7 @@  struct usb_context {
 #define EVENT_DEV_ASLEEP		7
 #define EVENT_DEV_OPEN			8
 #define EVENT_STAT_UPDATE		9
+#define EVENT_DEV_DISCONNECT		10
 
 struct statstage {
 	struct mutex			access_lock;	/* for stats access */
@@ -605,9 +606,14 @@  static int lan78xx_alloc_tx_resources(struct lan78xx_net *dev)
 
 static int lan78xx_read_reg(struct lan78xx_net *dev, u32 index, u32 *data)
 {
-	u32 *buf = kmalloc(sizeof(u32), GFP_KERNEL);
+	u32 *buf;
 	int ret;
 
+	if (test_bit(EVENT_DEV_DISCONNECT, &dev->flags))
+		return -ENODEV;
+
+	buf = kmalloc(sizeof(u32), GFP_KERNEL);
+
 	if (!buf)
 		return -ENOMEM;
 
@@ -631,9 +637,13 @@  static int lan78xx_read_reg(struct lan78xx_net *dev, u32 index, u32 *data)
 
 static int lan78xx_write_reg(struct lan78xx_net *dev, u32 index, u32 data)
 {
-	u32 *buf = kmalloc(sizeof(u32), GFP_KERNEL);
+	u32 *buf;
 	int ret;
 
+	if (test_bit(EVENT_DEV_DISCONNECT, &dev->flags))
+		return -ENODEV;
+
+	buf = kmalloc(sizeof(u32), GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
@@ -2972,6 +2982,8 @@  static int lan78xx_open(struct net_device *net)
 	struct lan78xx_net *dev = netdev_priv(net);
 	int ret;
 
+	netif_dbg(dev, ifup, dev->net, "open device");
+
 	ret = usb_autopm_get_interface(dev->intf);
 	if (ret < 0)
 		goto out;
@@ -3059,6 +3071,8 @@  static int lan78xx_stop(struct net_device *net)
 {
 	struct lan78xx_net *dev = netdev_priv(net);
 
+	netif_dbg(dev, ifup, dev->net, "stop device");
+
 	mutex_lock(&dev->dev_mutex);
 
 	if (timer_pending(&dev->stat_monitor))
@@ -3087,7 +3101,11 @@  static int lan78xx_stop(struct net_device *net)
 	 * can't flush_scheduled_work() until we drop rtnl (later),
 	 * else workers could deadlock; so make workers a NOP.
 	 */
-	dev->flags = 0;
+	clear_bit(EVENT_TX_HALT, &dev->flags);
+	clear_bit(EVENT_RX_HALT, &dev->flags);
+	clear_bit(EVENT_LINK_RESET, &dev->flags);
+	clear_bit(EVENT_STAT_UPDATE, &dev->flags);
+
 	cancel_delayed_work_sync(&dev->wq);
 
 	usb_autopm_put_interface(dev->intf);
@@ -3967,6 +3985,9 @@  static void lan78xx_delayedwork(struct work_struct *work)
 
 	dev = container_of(work, struct lan78xx_net, wq.work);
 
+	if (test_bit(EVENT_DEV_DISCONNECT, &dev->flags))
+		return;
+
 	if (usb_autopm_get_interface(dev->intf) < 0)
 		return;
 
@@ -4093,10 +4114,18 @@  static void lan78xx_disconnect(struct usb_interface *intf)
 	if (!dev)
 		return;
 
+	set_bit(EVENT_DEV_DISCONNECT, &dev->flags);
+
 	netif_napi_del(&dev->napi);
 
 	udev = interface_to_usbdev(intf);
+
 	net = dev->net;
+
+	unregister_netdev(net);
+
+	cancel_delayed_work_sync(&dev->wq);
+
 	phydev = net->phydev;
 
 	phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);
@@ -4107,10 +4136,6 @@  static void lan78xx_disconnect(struct usb_interface *intf)
 	if (phy_is_pseudo_fixed_link(phydev))
 		fixed_phy_unregister(phydev);
 
-	unregister_netdev(net);
-
-	cancel_delayed_work_sync(&dev->wq);
-
 	usb_scuttle_anchored_urbs(&dev->deferred);
 
 	if (timer_pending(&dev->stat_monitor))