lkml.org 
[lkml]   [2022]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 037/545] usbnet: Fix linkwatch use-after-free on disconnect
    Date
    From: Lukas Wunner <lukas@wunner.de>

    commit a69e617e533edddf3fa3123149900f36e0a6dc74 upstream.

    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@google.com>
    Link: https://lore.kernel.org/netdev/CAG48ez0MHBbENX5gCdHAUXZ7h7s20LnepBF-pa5M=7Bi-jZrEA@mail.gmail.com/
    Reported-by: Oleksij Rempel <o.rempel@pengutronix.de>
    Link: https://lore.kernel.org/netdev/20220315113841.GA22337@pengutronix.de/
    Signed-off-by: Lukas Wunner <lukas@wunner.de>
    Cc: stable@vger.kernel.org
    Acked-by: Oliver Neukum <oneukum@suse.com>
    Link: https://lore.kernel.org/r/d1c87ebe9fc502bffcd1576e238d685ad08321e4.1655987888.git.lukas@wunner.de
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/net/usb/usbnet.c | 8 ++------
    1 file changed, 2 insertions(+), 6 deletions(-)

    --- a/drivers/net/usb/usbnet.c
    +++ b/drivers/net/usb/usbnet.c
    @@ -830,13 +830,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);

    @@ -1585,8 +1583,6 @@ void usbnet_disconnect (struct usb_inter
    net = dev->net;
    unregister_netdev (net);

    - cancel_work_sync(&dev->kevent);
    -
    usb_scuttle_anchored_urbs(&dev->deferred);

    if (dev->driver_info->unbind)

    \
     
     \ /
      Last update: 2022-08-19 17:49    [W:3.214 / U:2.652 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site