lan78xx: Fix race condition in disconnect handling

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 a kernel lock up due to a null pointer dereference in the
driver's deferred work handling routine lan78xx_delayedwork().
The following changes fix this problem.

Add new status flag EVENT_DEV_DISCONNECT to indicate when the
device has been removed and use it to prevent operations, such as
register access, that will fail once the device is removed.

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 cancelled.

Signed-off-by: John Efstathiades <john.efstathiades@pebblebay.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9f395504f77e082ab43a90b1174fbdce5a3ce3ee..4ec752d9751a9fccd7ede5615c79a52dd0719fc7 100644 (file)
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -360,6 +360,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 */
@@ -444,9 +445,13 @@ MODULE_PARM_DESC(msg_level, "Override default message level");
 
 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;
 
@@ -470,9 +475,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;
 
@@ -3146,16 +3155,23 @@ static void tx_complete(struct urb *urb)
                /* software-driven interface shutdown */
                case -ECONNRESET:
                case -ESHUTDOWN:
+                       netif_dbg(dev, tx_err, dev->net,
+                                 "tx err interface gone %d\n",
+                                 entry->urb->status);
                        break;
 
                case -EPROTO:
                case -ETIME:
                case -EILSEQ:
                        netif_stop_queue(dev->net);
+                       netif_dbg(dev, tx_err, dev->net,
+                                 "tx err queue stopped %d\n",
+                                 entry->urb->status);
                        break;
                default:
                        netif_dbg(dev, tx_err, dev->net,
-                                 "tx err %d\n", entry->urb->status);
+                                 "unknown tx err %d\n",
+                                 entry->urb->status);
                        break;
                }
        }
@@ -3489,6 +3505,7 @@ static int rx_submit(struct lan78xx_net *dev, struct urb *urb, gfp_t flags)
                        lan78xx_defer_kevent(dev, EVENT_RX_HALT);
                        break;
                case -ENODEV:
+               case -ENOENT:
                        netif_dbg(dev, ifdown, dev->net, "device gone\n");
                        netif_device_detach(dev->net);
                        break;
@@ -3689,6 +3706,12 @@ gso_skb:
                lan78xx_defer_kevent(dev, EVENT_TX_HALT);
                usb_autopm_put_interface_async(dev->intf);
                break;
+       case -ENODEV:
+       case -ENOENT:
+               netif_dbg(dev, tx_err, dev->net,
+                         "tx: submit urb err %d (disconnected?)", ret);
+               netif_device_detach(dev->net);
+               break;
        default:
                usb_autopm_put_interface_async(dev->intf);
                netif_dbg(dev, tx_err, dev->net,
@@ -3783,6 +3806,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;
 
@@ -3857,6 +3883,7 @@ static void intr_complete(struct urb *urb)
 
        /* software-driven interface shutdown */
        case -ENOENT:                   /* urb killed */
+       case -ENODEV:                   /* hardware gone */
        case -ESHUTDOWN:                /* hardware gone */
                netif_dbg(dev, ifdown, dev->net,
                          "intr shutdown, code %d\n", status);
@@ -3870,14 +3897,29 @@ static void intr_complete(struct urb *urb)
                break;
        }
 
-       if (!netif_running(dev->net))
+       if (!netif_device_present(dev->net) ||
+           !netif_running(dev->net)) {
+               netdev_warn(dev->net, "not submitting new status URB");
                return;
+       }
 
        memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
        status = usb_submit_urb(urb, GFP_ATOMIC);
-       if (status != 0)
+
+       switch (status) {
+       case  0:
+               break;
+       case -ENODEV:
+       case -ENOENT:
+               netif_dbg(dev, timer, dev->net,
+                         "intr resubmit %d (disconnect?)", status);
+               netif_device_detach(dev->net);
+               break;
+       default:
                netif_err(dev, timer, dev->net,
                          "intr resubmit --> %d\n", status);
+               break;
+       }
 }
 
 static void lan78xx_disconnect(struct usb_interface *intf)
@@ -3892,8 +3934,15 @@ static void lan78xx_disconnect(struct usb_interface *intf)
        if (!dev)
                return;
 
+       set_bit(EVENT_DEV_DISCONNECT, &dev->flags);
+
        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);
@@ -3904,12 +3953,11 @@ 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))
+               del_timer_sync(&dev->stat_monitor);
+
        lan78xx_unbind(dev, intf);
 
        usb_kill_urb(dev->urb_intr);