Patch "usbnet: Fix linkwatch use-after-free on disconnect" has been added to the 5.19-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    usbnet: Fix linkwatch use-after-free on disconnect

to the 5.19-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     usbnet-fix-linkwatch-use-after-free-on-disconnect.patch
and it can be found in the queue-5.19 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit e76a4cdb309afe50d707c3a04df9f64540323d98
Author: Lukas Wunner <lukas@xxxxxxxxx>
Date:   Thu Jun 23 14:50:59 2022 +0200

    usbnet: Fix linkwatch use-after-free on disconnect
    
    [ Upstream commit a69e617e533edddf3fa3123149900f36e0a6dc74 ]
    
    usbnet uses the work usbnet_deferred_kevent() to perform tasks which may
    sleep.  On disconnect, completion of the work was originally awaited in
    ->ndo_stop().  But in 2003, that was moved to ->disconnect() by historic
    commit "[PATCH] USB: usbnet, prevent exotic rtnl deadlock":
    
      https://git.kernel.org/tglx/history/c/0f138bbfd83c
    
    The change was made because back then, the kernel's workqueue
    implementation did not allow waiting for a single work.  One had to wait
    for completion of *all* work by calling flush_scheduled_work(), and that
    could deadlock when waiting for usbnet_deferred_kevent() with rtnl_mutex
    held in ->ndo_stop().
    
    The commit solved one problem but created another:  It causes a
    use-after-free in USB Ethernet drivers aqc111.c, asix_devices.c,
    ax88179_178a.c, ch9200.c and smsc75xx.c:
    
    * If the drivers receive a link change interrupt immediately before
      disconnect, they raise EVENT_LINK_RESET in their (non-sleepable)
      ->status() callback and schedule usbnet_deferred_kevent().
    * usbnet_deferred_kevent() invokes the driver's ->link_reset() callback,
      which calls netif_carrier_{on,off}().
    * That in turn schedules the work linkwatch_event().
    
    Because usbnet_deferred_kevent() is awaited after unregister_netdev(),
    netif_carrier_{on,off}() may operate on an unregistered netdev and
    linkwatch_event() may run after free_netdev(), causing a use-after-free.
    
    In 2010, usbnet was changed to only wait for a single instance of
    usbnet_deferred_kevent() instead of *all* work by commit 23f333a2bfaf
    ("drivers/net: don't use flush_scheduled_work()").
    
    Unfortunately the commit neglected to move the wait back to
    ->ndo_stop().  Rectify that omission at long last.
    
    Reported-by: Jann Horn <jannh@xxxxxxxxxx>
    Link: https://lore.kernel.org/netdev/CAG48ez0MHBbENX5gCdHAUXZ7h7s20LnepBF-pa5M=7Bi-jZrEA@xxxxxxxxxxxxxx/
    Reported-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
    Link: https://lore.kernel.org/netdev/20220315113841.GA22337@xxxxxxxxxxxxxx/
    Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
    Cc: stable@xxxxxxxxxxxxxxx
    Acked-by: Oliver Neukum <oneukum@xxxxxxxx>
    Link: https://lore.kernel.org/r/d1c87ebe9fc502bffcd1576e238d685ad08321e4.1655987888.git.lukas@xxxxxxxxx
    Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 78a92751ce4c..0ed09bb91c44 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -849,13 +849,11 @@ int usbnet_stop (struct net_device *net)
 
 	mpn = !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
 
-	/* deferred work (task, timer, softirq) must also stop.
-	 * can't flush_scheduled_work() until we drop rtnl (later),
-	 * else workers could deadlock; so make workers a NOP.
-	 */
+	/* deferred work (timer, softirq, task) must also stop */
 	dev->flags = 0;
 	del_timer_sync (&dev->delay);
 	tasklet_kill (&dev->bh);
+	cancel_work_sync(&dev->kevent);
 	if (!pm)
 		usb_autopm_put_interface(dev->intf);
 
@@ -1619,8 +1617,6 @@ void usbnet_disconnect (struct usb_interface *intf)
 	net = dev->net;
 	unregister_netdev (net);
 
-	cancel_work_sync(&dev->kevent);
-
 	usb_scuttle_anchored_urbs(&dev->deferred);
 
 	if (dev->driver_info->unbind)



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux