From patchwork Wed Apr 27 05:48:01 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 567067 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 94EB0C433F5 for ; Wed, 27 Apr 2022 05:57:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357932AbiD0GAw (ORCPT ); Wed, 27 Apr 2022 02:00:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60520 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357916AbiD0GAv (ORCPT ); Wed, 27 Apr 2022 02:00:51 -0400 X-Greylist: delayed 490 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 26 Apr 2022 22:57:41 PDT Received: from mailout1.hostsharing.net (mailout1.hostsharing.net [83.223.95.204]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4E4792DD57; Tue, 26 Apr 2022 22:57:41 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [IPv6:2a01:37:1000::53df:5f1c:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS DV RSA Mixed SHA256 2020 CA-1" (verified OK)) by mailout1.hostsharing.net (Postfix) with ESMTPS id DBB1110057928; Wed, 27 Apr 2022 07:57:39 +0200 (CEST) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id AA58E61ABA99; Wed, 27 Apr 2022 07:57:39 +0200 (CEST) X-Mailbox-Line: From ca65428c57c0b15ead00c92decb9b4a01a0fa81f Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Lukas Wunner Date: Wed, 27 Apr 2022 07:48:01 +0200 Subject: [PATCH net-next 1/7] usbnet: Run unregister_netdev() before unbind() again To: Steve Glendinning , UNGLinuxDriver@microchip.com, Oliver Neukum , "David S. Miller" , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, Andre Edich , Oleksij Rempel , Martyn Welch , Gabriel Hojda , Christoph Fritz , Lino Sanfilippo , Philipp Rosenberger , Heiner Kallweit , Andrew Lunn , Russell King Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org 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(). Signed-off-by: Lukas Wunner Cc: Oleksij Rempel Cc: Martyn Welch Cc: Andrew Lunn --- 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); From patchwork Wed Apr 27 05:48:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 567066 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 E283BC433FE for ; Wed, 27 Apr 2022 06:01:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357952AbiD0GEt (ORCPT ); Wed, 27 Apr 2022 02:04:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1348733AbiD0GEt (ORCPT ); Wed, 27 Apr 2022 02:04:49 -0400 Received: from mailout3.hostsharing.net (mailout3.hostsharing.net [IPv6:2a01:4f8:150:2161:1:b009:f236:0]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 530D444A1E; Tue, 26 Apr 2022 23:01:39 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS DV RSA Mixed SHA256 2020 CA-1" (verified OK)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 9E867100B0166; Wed, 27 Apr 2022 08:01:35 +0200 (CEST) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 5DADE61ABA99; Wed, 27 Apr 2022 08:01:35 +0200 (CEST) X-Mailbox-Line: From cf64096387a685596b3590452b899618d46de433 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: References: From: Lukas Wunner Date: Wed, 27 Apr 2022 07:48:03 +0200 Subject: [PATCH net-next 3/7] usbnet: smsc95xx: Don't reset PHY behind PHY driver's back To: Steve Glendinning , UNGLinuxDriver@microchip.com, Oliver Neukum , "David S. Miller" , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, Andre Edich , Oleksij Rempel , Martyn Welch , Gabriel Hojda , Christoph Fritz , Lino Sanfilippo , Philipp Rosenberger , Heiner Kallweit , Andrew Lunn , Russell King Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org smsc95xx_reset() resets the PHY behind the PHY driver's back, which seems like a bad idea generally. Remove that portion of the function. We're about to use PHY interrupts instead of polling to detect link changes on SMSC LAN95xx chips. Because smsc95xx_reset() is called from usbnet_open(), PHY interrupt settings are lost whenever the net_device is brought up. There are two other callers of smsc95xx_reset(), namely smsc95xx_bind() and smsc95xx_reset_resume(), and both may indeed benefit from a PHY reset. However they already perform one through their calls to phy_connect_direct() and phy_init_hw(). Signed-off-by: Lukas Wunner Cc: Martyn Welch Cc: Gabriel Hojda --- drivers/net/usb/smsc95xx.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 2cb44d65bbc3..6c37c7adde1b 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -887,24 +887,6 @@ static int smsc95xx_reset(struct usbnet *dev) return ret; } - ret = smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_); - if (ret < 0) - return ret; - - timeout = 0; - do { - msleep(10); - ret = smsc95xx_read_reg(dev, PM_CTRL, &read_buf); - if (ret < 0) - return ret; - timeout++; - } while ((read_buf & PM_CTL_PHY_RST_) && (timeout < 100)); - - if (timeout >= 100) { - netdev_warn(dev->net, "timeout waiting for PHY Reset\n"); - return ret; - } - ret = smsc95xx_set_mac_address(dev); if (ret < 0) return ret; From patchwork Wed Apr 27 05:48:04 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 567065 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 117CFC433F5 for ; Wed, 27 Apr 2022 06:10:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357986AbiD0GNc (ORCPT ); Wed, 27 Apr 2022 02:13:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51038 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1357985AbiD0GNa (ORCPT ); Wed, 27 Apr 2022 02:13:30 -0400 X-Greylist: delayed 347 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Tue, 26 Apr 2022 23:10:18 PDT Received: from mailout2.hostsharing.net (mailout2.hostsharing.net [IPv6:2a01:37:3000::53df:4ee9:0]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 26703222BD; Tue, 26 Apr 2022 23:10:16 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS DV RSA Mixed SHA256 2020 CA-1" (verified OK)) by mailout2.hostsharing.net (Postfix) with ESMTPS id F07E910321188; Wed, 27 Apr 2022 08:04:25 +0200 (CEST) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id C4D1861ABA99; Wed, 27 Apr 2022 08:04:25 +0200 (CEST) X-Mailbox-Line: From 28fd5a3d53a401cdf8ae0a116108702a46d2c7a2 Mon Sep 17 00:00:00 2001 Message-Id: <28fd5a3d53a401cdf8ae0a116108702a46d2c7a2.1651037513.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Wed, 27 Apr 2022 07:48:04 +0200 Subject: [PATCH net-next 4/7] usbnet: smsc95xx: Avoid link settings race on interrupt reception To: Steve Glendinning , UNGLinuxDriver@microchip.com, Oliver Neukum , "David S. Miller" , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, Andre Edich , Oleksij Rempel , Martyn Welch , Gabriel Hojda , Christoph Fritz , Lino Sanfilippo , Philipp Rosenberger , Heiner Kallweit , Andrew Lunn , Russell King Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org When a PHY interrupt is signaled, the SMSC LAN95xx driver updates the MAC full duplex mode and PHY flow control registers based on cached data in struct phy_device: smsc95xx_status() # raises EVENT_LINK_RESET usbnet_deferred_kevent() smsc95xx_link_reset() # uses cached data in phydev Simultaneously, phylib polls link status once per second and updates that cached data: phy_state_machine() phy_check_link_status() phy_read_status() lan87xx_read_status() genphy_read_status() # updates cached data in phydev If smsc95xx_link_reset() wins the race against genphy_read_status(), the registers may be updated based on stale data. E.g. if the link was previously down, phydev->duplex is set to DUPLEX_UNKNOWN and that's what smsc95xx_link_reset() will use, even though genphy_read_status() may update it to DUPLEX_FULL afterwards. PHY interrupts are currently only enabled on suspend to trigger wakeup, so the impact of the race is limited, but we're about to enable them perpetually. Avoid the race by delaying execution of smsc95xx_link_reset() until phy_state_machine() has done its job and calls back via smsc95xx_handle_link_change(). Signaling EVENT_LINK_RESET on wakeup is not necessary because phylib picks up link status changes through polling. So drop the declaration of a ->link_reset() callback. Signed-off-by: Lukas Wunner --- drivers/net/usb/smsc95xx.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 6c37c7adde1b..36abfaeae3c6 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -566,7 +566,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev) return smsc95xx_write_reg(dev, AFC_CFG, afc_cfg); } -static int smsc95xx_link_reset(struct usbnet *dev) +static void smsc95xx_mac_update_fullduplex(struct usbnet *dev) { struct smsc95xx_priv *pdata = dev->driver_priv; unsigned long flags; @@ -583,14 +583,14 @@ static int smsc95xx_link_reset(struct usbnet *dev) spin_unlock_irqrestore(&pdata->mac_cr_lock, flags); ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr); - if (ret < 0) - return ret; + if (ret < 0) { + netdev_warn(dev->net, "Error updating MAC full duplex mode\n"); + return; + } ret = smsc95xx_phy_update_flowcontrol(dev); if (ret < 0) netdev_warn(dev->net, "Error updating PHY flow control\n"); - - return ret; } static void smsc95xx_status(struct usbnet *dev, struct urb *urb) @@ -607,7 +607,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb) netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata); if (intdata & INT_ENP_PHY_INT_) - usbnet_defer_kevent(dev, EVENT_LINK_RESET); + ; else netdev_warn(dev->net, "unexpected interrupt, intdata=0x%08X\n", intdata); @@ -1070,6 +1070,7 @@ static void smsc95xx_handle_link_change(struct net_device *net) struct usbnet *dev = netdev_priv(net); phy_print_status(net->phydev); + smsc95xx_mac_update_fullduplex(dev); usbnet_defer_kevent(dev, EVENT_LINK_CHANGE); } @@ -1975,7 +1976,6 @@ static const struct driver_info smsc95xx_info = { .description = "smsc95xx USB 2.0 Ethernet", .bind = smsc95xx_bind, .unbind = smsc95xx_unbind, - .link_reset = smsc95xx_link_reset, .reset = smsc95xx_reset, .check_connect = smsc95xx_start_phy, .stop = smsc95xx_stop,