From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 509CFC433EF for ; Mon, 30 May 2022 13:37:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237694AbiE3NhZ (ORCPT ); Mon, 30 May 2022 09:37:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56420 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238182AbiE3NgK (ORCPT ); Mon, 30 May 2022 09:36:10 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66E64986CF; Mon, 30 May 2022 06:29:54 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id E26FDB80D89; Mon, 30 May 2022 13:29:52 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8BA96C3411C; Mon, 30 May 2022 13:29:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1653917391; bh=67f36S+4rjT+JdsrzBc14AqQKTdEkAdczFOtEskmpRI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f4TkQrnqFbA414/oVIi7K9Wxu9EcpZ8cm2kNnEIb7oPzCwLvZ6WJ6SGG20oATPmNe KSJcq3ZbstAsoF+6Ku/AEAVFlCjcyOQi2gsv0qP7nSVjpq+uuxImSFnFdddUs72YsE AhvQBY8VOvjfpeZTFBVbyR6jc3DJ8y/3ZaIX2DyH7rUcbjXOR3kLiXcT8TvhwNbNqg uevFeBX2vw8c00+NZC8Yw3WmJjPZbWlfXGL8bL9Wzl1I6251TA/pFf60YMnyQ7p6Gx 3c1Ndvh2WkOTwirrfGzi2yweKebWFkE/dVcgY2hWEpOkhXKjYfrZsP9FmgHUwpQDdq jJOIIsjF4Q4EA== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Lukas Wunner , Oleksij Rempel , Ferry Toth , Oliver Neukum , Martyn Welch , Andrew Lunn , "David S . Miller" , Sasha Levin , edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, steve.glendinning@shawell.net, UNGLinuxDriver@microchip.com, linux@rempel-privat.de, colin.king@intel.com, paskripkin@gmail.com, linux-usb@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH AUTOSEL 5.18 118/159] usbnet: Run unregister_netdev() before unbind() again Date: Mon, 30 May 2022 09:23:43 -0400 Message-Id: <20220530132425.1929512-118-sashal@kernel.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220530132425.1929512-1-sashal@kernel.org> References: <20220530132425.1929512-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Lukas Wunner [ 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 # LAN9514/9512/9500 Tested-by: Ferry Toth # LAN9514 Signed-off-by: Lukas Wunner Acked-by: Oliver Neukum Cc: Martyn Welch Cc: Andrew Lunn Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- 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 38e47a93fb83..5b5eb630c4b7 100644 --- a/drivers/net/usb/asix_devices.c +++ b/drivers/net/usb/asix_devices.c @@ -795,11 +795,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 4ef61f6b85df..edf0492ad489 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -1243,8 +1243,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