lkml.org 
[lkml]   [2022]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.17 146/772] usbnet: Run unregister_netdev() before unbind() again
    Date
    From: Lukas Wunner <lukas@wunner.de>

    [ Upstream commit d1408f6b4dd78fb1b9e26bcf64477984e5f85409 ]

    Commit 2c9d6c2b871d ("usbnet: run unbind() before unregister_netdev()")
    sought to fix a use-after-free on disconnect of USB Ethernet adapters.

    It turns out that a different fix is necessary to address the issue:
    https://lore.kernel.org/netdev/18b3541e5372bc9b9fc733d422f4e698c089077c.1650177997.git.lukas@wunner.de/

    So the commit was not necessary.

    The commit made binding and unbinding of USB Ethernet asymmetrical:
    Before, usbnet_probe() first invoked the ->bind() callback and then
    register_netdev(). usbnet_disconnect() mirrored that by first invoking
    unregister_netdev() and then ->unbind().

    Since the commit, the order in usbnet_disconnect() is reversed and no
    longer mirrors usbnet_probe().

    One consequence is that a PHY disconnected (and stopped) in ->unbind()
    is afterwards stopped once more by unregister_netdev() as it closes the
    netdev before unregistering. That necessitates a contortion in ->stop()
    because the PHY may only be stopped if it hasn't already been
    disconnected.

    Reverting the commit allows making the call to phy_stop() unconditional
    in ->stop().

    Tested-by: Oleksij Rempel <o.rempel@pengutronix.de> # LAN9514/9512/9500
    Tested-by: Ferry Toth <fntoth@gmail.com> # LAN9514
    Signed-off-by: Lukas Wunner <lukas@wunner.de>
    Acked-by: Oliver Neukum <oneukum@suse.com>
    Cc: Martyn Welch <martyn.welch@collabora.com>
    Cc: Andrew Lunn <andrew@lunn.ch>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/net/usb/asix_devices.c | 6 +-----
    drivers/net/usb/smsc95xx.c | 3 +--
    drivers/net/usb/usbnet.c | 6 +++---
    3 files changed, 5 insertions(+), 10 deletions(-)

    diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
    index 6b2fbdf4e0fd..34854ee537dc 100644
    --- a/drivers/net/usb/asix_devices.c
    +++ b/drivers/net/usb/asix_devices.c
    @@ -799,11 +799,7 @@ static int ax88772_stop(struct usbnet *dev)
    {
    struct asix_common_private *priv = dev->driver_priv;

    - /* On unplugged USB, we will get MDIO communication errors and the
    - * PHY will be set in to PHY_HALTED state.
    - */
    - if (priv->phydev->state != PHY_HALTED)
    - phy_stop(priv->phydev);
    + phy_stop(priv->phydev);

    return 0;
    }
    diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
    index a0f29482294d..d81485cd7e90 100644
    --- a/drivers/net/usb/smsc95xx.c
    +++ b/drivers/net/usb/smsc95xx.c
    @@ -1218,8 +1218,7 @@ static int smsc95xx_start_phy(struct usbnet *dev)

    static int smsc95xx_stop(struct usbnet *dev)
    {
    - if (dev->net->phydev)
    - phy_stop(dev->net->phydev);
    + phy_stop(dev->net->phydev);

    return 0;
    }
    diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
    index 9a6450f796dc..36b24ec11650 100644
    --- a/drivers/net/usb/usbnet.c
    +++ b/drivers/net/usb/usbnet.c
    @@ -1616,9 +1616,6 @@ void usbnet_disconnect (struct usb_interface *intf)
    xdev->bus->bus_name, xdev->devpath,
    dev->driver_info->description);

    - if (dev->driver_info->unbind)
    - dev->driver_info->unbind(dev, intf);
    -
    net = dev->net;
    unregister_netdev (net);

    @@ -1626,6 +1623,9 @@ void usbnet_disconnect (struct usb_interface *intf)

    usb_scuttle_anchored_urbs(&dev->deferred);

    + if (dev->driver_info->unbind)
    + dev->driver_info->unbind(dev, intf);
    +
    usb_kill_urb(dev->interrupt);
    usb_free_urb(dev->interrupt);
    kfree(dev->padding_pkt);
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-06-07 22:43    [W:2.675 / U:0.632 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site