| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <12992128.uLZWGnKmhe@steina-w>
Date: Wed, 09 Mar 2022 15:02:21 +0100
From: Alexander Stein <alexander.stein@...tq-group.com>
To: kuba@...nel.org, Fabio Estevam <festevam@...il.com>
Cc: steve.glendinning@...well.net, UNGLinuxDriver@...rochip.com,
fntoth@...il.com, martyn.welch@...labora.com, andrew@...n.ch,
netdev@...r.kernel.org, linux-usb@...r.kernel.org, marex@...x.de,
Fabio Estevam <festevam@...x.de>
Subject: Re: (EXT) [PATCH v2 net] smsc95xx: Ignore -ENODEV errors when device is unplugged
Am Samstag, 5. März 2022, 21:47:20 CET schrieb Fabio Estevam:
> From: Fabio Estevam <festevam@...x.de>
>
> According to Documentation/driver-api/usb/URB.rst when a device
> is unplugged usb_submit_urb() returns -ENODEV.
>
> This error code propagates all the way up to usbnet_read_cmd() and
> usbnet_write_cmd() calls inside the smsc95xx.c driver during
> Ethernet cable unplug, unbind or reboot.
>
> This causes the following errors to be shown on reboot, for example:
>
> ci_hdrc ci_hdrc.1: remove, state 1
> usb usb2: USB disconnect, device number 1
> usb 2-1: USB disconnect, device number 2
> usb 2-1.1: USB disconnect, device number 3
> smsc95xx 2-1.1:1.0 eth1: unregister 'smsc95xx' usb-ci_hdrc.1-1.1, smsc95xx
> USB 2.0 Ethernet smsc95xx 2-1.1:1.0 eth1: Failed to read reg index
> 0x00000114: -19
> smsc95xx 2-1.1:1.0 eth1: Error reading MII_ACCESS
> smsc95xx 2-1.1:1.0 eth1: __smsc95xx_mdio_read: MII is busy
> smsc95xx 2-1.1:1.0 eth1: Failed to read reg index 0x00000114: -19
> smsc95xx 2-1.1:1.0 eth1: Error reading MII_ACCESS
> smsc95xx 2-1.1:1.0 eth1: __smsc95xx_mdio_read: MII is busy
> smsc95xx 2-1.1:1.0 eth1: hardware isn't capable of remote wakeup
> usb 2-1.4: USB disconnect, device number 4
> ci_hdrc ci_hdrc.1: USB bus 2 deregistered
> ci_hdrc ci_hdrc.0: remove, state 4
> usb usb1: USB disconnect, device number 1
> ci_hdrc ci_hdrc.0: USB bus 1 deregistered
> imx2-wdt 30280000.watchdog: Device shutdown: Expect reboot!
> reboot: Restarting system
>
> Ignore the -ENODEV errors inside __smsc95xx_mdio_read() and
> __smsc95xx_phy_wait_not_busy() and do not print error messages
> when -ENODEV is returned.
>
> Fixes: a049a30fc27c ("net: usb: Correct PHY handling of smsc95xx")
> Signed-off-by: Fabio Estevam <festevam@...x.de>
Oh BTW, is this queued for stable? Which versions? If 'Fixes: a049a30fc27c
("net: usb: Correct PHY handling of smsc95xx")' is the indicator, it's not
enough. This errors also shows up on v5.15.27 and is fixed with this patch.
Regards,
Alexander
> ---
> Changes since v1:
> - Added 'net' annotation - Andrew
> - Added Fixes tag - Andrew
> - Avoided undefined 'buf' behaviour in __smsc95xx_read_reg() - Andrew
>
> drivers/net/usb/smsc95xx.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index b17bff6a1015..e5b744851146 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -84,9 +84,10 @@ static int __must_check __smsc95xx_read_reg(struct usbnet
> *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN
>
> | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
>
> 0, index, &buf, 4);
> - if (unlikely(ret < 0)) {
> - netdev_warn(dev->net, "Failed to read reg index 0x%08x:
%d\n",
> - index, ret);
> + if (ret < 0) {
> + if (ret != -ENODEV)
> + netdev_warn(dev->net, "Failed to read reg
index 0x%08x: %d\n",
> + index, ret);
> return ret;
> }
>
> @@ -116,7 +117,7 @@ static int __must_check __smsc95xx_write_reg(struct
> usbnet *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
> USB_DIR_OUT
>
> | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
>
> 0, index, &buf, 4);
> - if (unlikely(ret < 0))
> + if (ret < 0 && ret != -ENODEV)
> netdev_warn(dev->net, "Failed to write reg index 0x%08x:
%d\n",
> index, ret);
>
> @@ -159,6 +160,9 @@ static int __must_check
> __smsc95xx_phy_wait_not_busy(struct usbnet *dev, do {
> ret = __smsc95xx_read_reg(dev, MII_ADDR, &val, in_pm);
> if (ret < 0) {
> + /* Ignore -ENODEV error during disconnect()
*/
> + if (ret == -ENODEV)
> + return 0;
> netdev_warn(dev->net, "Error reading
MII_ACCESS\n");
> return ret;
> }
> @@ -194,7 +198,8 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int
> phy_id, int idx, addr = mii_address_cmd(phy_id, idx, MII_READ_ |
> MII_BUSY_);
> ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
> if (ret < 0) {
> - netdev_warn(dev->net, "Error writing MII_ADDR\n");
> + if (ret != -ENODEV)
> + netdev_warn(dev->net, "Error writing
MII_ADDR\n");
> goto done;
> }
>
> @@ -206,7 +211,8 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int
> phy_id, int idx,
>
> ret = __smsc95xx_read_reg(dev, MII_DATA, &val, in_pm);
> if (ret < 0) {
> - netdev_warn(dev->net, "Error reading MII_DATA\n");
> + if (ret != -ENODEV)
> + netdev_warn(dev->net, "Error reading
MII_DATA\n");
> goto done;
> }
>
> @@ -214,6 +220,10 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int
> phy_id, int idx,
>
> done:
> mutex_unlock(&dev->phy_mutex);
> +
> + /* Ignore -ENODEV error during disconnect() */
> + if (ret == -ENODEV)
> + return 0;
> return ret;
> }
>
> @@ -235,7 +245,8 @@ static void __smsc95xx_mdio_write(struct usbnet *dev,
> int phy_id, val = regval;
> ret = __smsc95xx_write_reg(dev, MII_DATA, val, in_pm);
> if (ret < 0) {
> - netdev_warn(dev->net, "Error writing MII_DATA\n");
> + if (ret != -ENODEV)
> + netdev_warn(dev->net, "Error writing
MII_DATA\n");
> goto done;
> }
>
> @@ -243,7 +254,8 @@ static void __smsc95xx_mdio_write(struct usbnet *dev,
> int phy_id, addr = mii_address_cmd(phy_id, idx, MII_WRITE_ | MII_BUSY_);
> ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
> if (ret < 0) {
> - netdev_warn(dev->net, "Error writing MII_ADDR\n");
> + if (ret != -ENODEV)
> + netdev_warn(dev->net, "Error writing
MII_ADDR\n");
> goto done;
> }
Powered by blists - more mailing lists