[PATCH net 0/2] net: usb: smsc95xx: propagate MDIO errors

[PATCH net 0/2] net: usb: smsc95xx: propagate MDIO errors

[Artem Novak]

This series fixes error handling in the SMSC95xx MDIO operations.

The MDIO bus reset callback currently reports success even when accessing
the PHY reset control register fails. The MDIO write helper also
discards register access errors, causing the MDIO bus callback
to report success when a write fails.

The patches address these two issues:

Return register access errors from the external PHY reset callback.

&

Propagate MDIO write errors through the MDIO bus callback,
while preserving the existing handling of -ENODEV during disconnect.

Both changes improve error reporting to the MDIO/PHYLIB layer.

Artem Novak (2):
  net: usb: smsc95xx: propagate MDIO reset errors
  net: usb: smsc95xx: propagate MDIO write errors

 drivers/net/usb/smsc95xx.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

-- 
2.56.0

[PATCH net 1/2] net: usb: smsc95xx: propagate MDIO reset errors

[Artem Novak]

The MDIO bus reset callback returns success even when reading or
writing the PHY reset control register fails.

Return the register access error so callers can detect a failed
PHY reset.

Fixes: 809ff97a677f ("net: usb: smsc95xx: fix external PHY reset")
Signed-off-by: Artem Novak <artem122novak@gmail.com>
---
 drivers/net/usb/smsc95xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 130a460c1f54..f85ee5f8a539 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -291,7 +291,7 @@ static int smsc95xx_mdiobus_reset(struct mii_bus *bus)
 reset_out:
 	mutex_unlock(&dev->phy_mutex);
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_mdiobus_read(struct mii_bus *bus, int phy_id, int idx)
-- 
2.56.0

Re: [PATCH net 1/2] net: usb: smsc95xx: propagate MDIO reset errors

[Andrew Lunn]

On Fri, Nov 13, 2026 at 11:09:53PM -0600, Artem Novak wrote:
> The MDIO bus reset callback returns success even when reading or
> writing the PHY reset control register fails.
> 
> Return the register access error so callers can detect a failed
> PHY reset.
> 
> Fixes: 809ff97a677f ("net: usb: smsc95xx: fix external PHY reset")
> Signed-off-by: Artem Novak <artem122novak@gmail.com>

Have you ever seen it fail?

net-next and no fixes tag please.

	 Andrew

Re: [PATCH net 1/2] net: usb: smsc95xx: propagate MDIO reset errors

[Artem Novak]

Hello,

Yes, I did observe a failed PHY reset on hardware,
but unfortunately I didn't capture logs or document the failure at the time.

I'll move the patch to net-next and remove the Fixes tag.

Thanks,
Artem

On Sat, Nov 14, 2026 at 4:08 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Nov 13, 2026 at 11:09:53PM -0600, Artem Novak wrote:
> > The MDIO bus reset callback returns success even when reading or
> > writing the PHY reset control register fails.
> >
> > Return the register access error so callers can detect a failed
> > PHY reset.
> >
> > Fixes: 809ff97a677f ("net: usb: smsc95xx: fix external PHY reset")
> > Signed-off-by: Artem Novak <artem122novak@gmail.com>
>
> Have you ever seen it fail?
>
> net-next and no fixes tag please.
>
>          Andrew

Re: [PATCH net 1/2] net: usb: smsc95xx: propagate MDIO reset errors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] net: usb: smsc95xx: smsc95xx_mdiobus_reset() returns 4 on
  success
--

commit 2539b52a07146a1a92198f65c494924038af75bf
Author: Artem Novak <artem122novak@gmail.com>

net: usb: smsc95xx: propagate MDIO reset errors

This commit changes smsc95xx_mdiobus_reset() to return ret instead of 0
so that register access errors during external PHY reset are propagated
to callers.

> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 130a460c1f546..f85ee5f8a5394 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -291,7 +291,7 @@ static int smsc95xx_mdiobus_reset(struct mii_bus *bus)
>  reset_out:
>  	mutex_unlock(&dev->phy_mutex);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int smsc95xx_mdiobus_read(struct mii_bus *bus, int phy_id, int idx)

[Severity: High]
Does smsc95xx_mdiobus_reset() now return 4 on success for devices with an
external PHY?

smsc95xx_write_reg() returns the number of bytes written (4) on success,
and ret is not reset to 0 before reaching reset_out.  __mdiobus_register()
treats any non-zero return from bus->reset() as an error and returns 4 to
smsc95xx_bind(), which unwinds and frees pdata (leaving dev->driver_priv
dangling) and returns 4 to usbnet_probe().  Because usbnet_probe() only
aborts when status < 0, probe continues and registers the net_device with
a freed dev->driver_priv and a NULL phydev, leading to a use-after-free
in subsequent driver callbacks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114050954.275495-1-artem122novak@gmail.com?part=1

Re: [PATCH net 1/2] net: usb: smsc95xx: propagate MDIO reset errors

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 1 · High: 0 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] The patch makes smsc95xx_mdiobus_reset()
  (drivers/net/usb/smsc95xx.c) return `ret` instead of 0.

--

net: usb: smsc95xx: propagate MDIO reset errors

This patch changes smsc95xx_mdiobus_reset() so it returns the result of
the PM_CTRL register accesses instead of always returning 0. The goal is
to report a failed external PHY reset to the caller.

> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 130a460c1f546..f85ee5f8a5394 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -291,7 +291,7 @@ static int smsc95xx_mdiobus_reset(struct mii_bus *bus)
>  reset_out:
>  	mutex_unlock(&dev->phy_mutex);
>  
> -	return 0;
> +	return ret;
>  }

[Severity: Critical]
What value does ret hold here when the reset succeeds?

smsc95xx_write_reg() passes back the result of usbnet_write_cmd()
unchanged. That result is the byte count from usb_control_msg():

drivers/net/usb/smsc95xx.c:smsc95xx_write_reg() {
    ...
	ret = fn(dev, USB_VENDOR_REQUEST_WRITE_REGISTER, USB_DIR_OUT
		 | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
		 0, index, &buf, 4);
	if (ret < 0 && ret != -ENODEV)
		netdev_warn(dev->net, "Failed to write reg index 0x%08x: %d\n",
			    index, ret);

	return ret;
}

smsc95xx_read_reg() also returns 4 on success. After a successful
external PHY reset, both "if (ret < 0)" checks pass, and the function
now returns 4 instead of 0.

__mdiobus_register() treats any non-zero value as a failure:

drivers/net/phy/mdio_bus_provider.c:__mdiobus_register() {
    ...
	if (bus->reset) {
		err = bus->reset(bus);
		if (err)
			goto error_reset_gpiod;
	}
    ...
}

It calls device_del() and returns 4. smsc95xx_bind() then takes its
error path:

	ret = mdiobus_register(pdata->mdiobus);
	if (ret) {
		netdev_err(dev->net, "Could not register MDIO bus\n");
		goto free_mdio;
	}

That path runs mdiobus_free(), irq_dispose_mapping(),
irq_domain_remove(), irq_domain_free_fwnode() and kfree(pdata). It
then returns 4 and leaves dev->driver_priv pointing at the freed pdata.

usbnet_probe() only aborts on negative values:

	status = info->bind(dev, udev);
	if (status < 0)
		goto out1;

Probe therefore goes on to register_netdev() with these leftovers:

  - dev->driver_priv pointing at freed memory
  - a freed mii_bus
  - a NULL dev->net->phydev

Wouldn't later driver callbacks then use pdata after it was freed? One
example is the pdata->pm_task dereference in smsc95xx_read_reg() and
smsc95xx_write_reg().

smsc95xx_unbind() runs on disconnect, or when probe fails after bind
and calls info->unbind. In that case it would:

  - call phy_disconnect() on a NULL phydev
  - call mdiobus_unregister() and mdiobus_free() on the freed mii_bus
  - free the irqdomain and fwnode a second time
  - call kfree(pdata) a second time

Can this double free pdata?

This path is taken whenever HW_CFG_PSEL_ is set. That seems to break
every probe of a device with an external PHY, which is the setup
809ff97a677f was fixing. The bit is read from the device, so an
emulated or malicious USB device could trigger this as well.

The next patch in the series, "net: usb: smsc95xx: propagate MDIO write
errors", does not touch smsc95xx_mdiobus_reset(). The problem would
still be there at the end of the series.

Would something like "return ret < 0 ? ret : 0;", or setting ret = 0
after the successful write, keep the error propagation and still return
0 on success?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114050954.275495-1-artem122novak%40gmail.com

[PATCH net 2/2] net: usb: smsc95xx: propagate MDIO write errors

[Artem Novak]

The MDIO write helper currently discards register access errors,
and the MDIO bus callback consequently reports success even when
a write fails.

Return the error from the helper and propagate it through the
MDIO bus callback. Continue treating -ENODEV as success during
device disconnect, consistent with the MDIO read path.

Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
Signed-off-by: Artem Novak <artem122novak@gmail.com>
---
 drivers/net/usb/smsc95xx.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index f85ee5f8a539..52035c160f4b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -215,8 +215,8 @@ static int smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx)
 	return ret;
 }
 
-static void smsc95xx_mdio_write(struct usbnet *dev, int phy_id, int idx,
-				int regval)
+static int smsc95xx_mdio_write(struct usbnet *dev, int phy_id, int idx,
+			       int regval)
 {
 	u32 val, addr;
 	int ret;
@@ -255,6 +255,12 @@ static void smsc95xx_mdio_write(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;
 }
 
 static int smsc95xx_mdiobus_reset(struct mii_bus *bus)
@@ -306,8 +312,7 @@ static int smsc95xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
 {
 	struct usbnet *dev = bus->priv;
 
-	smsc95xx_mdio_write(dev, phy_id, idx, regval);
-	return 0;
+	return smsc95xx_mdio_write(dev, phy_id, idx, regval);
 }
 
 static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
-- 
2.56.0

Re: [PATCH net 2/2] net: usb: smsc95xx: propagate MDIO write errors

[Andrew Lunn]

> Return the error from the helper and propagate it through the
> MDIO bus callback. Continue treating -ENODEV as success during
> device disconnect, consistent with the MDIO read path.

This is logically two different things, so should be two patches.

> Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")

net-next.

	Andrew

Re: [PATCH net 2/2] net: usb: smsc95xx: propagate MDIO write errors

[Artem Novak]

Thanks,
Artem

On Sat, Nov 14, 2026 at 4:10 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Return the error from the helper and propagate it through the
> > MDIO bus callback. Continue treating -ENODEV as success during
> > device disconnect, consistent with the MDIO read path.
>
> This is logically two different things, so should be two patches.
>
> > Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
>
> net-next.
>
>         Andrew

Re: [PATCH net 2/2] net: usb: smsc95xx: propagate MDIO write errors

[sashiko-bot]

> The MDIO write helper currently discards register access errors,
> and the MDIO bus callback consequently reports success even when
> a write fails.
> 
> Return the error from the helper and propagate it through the
> MDIO bus callback. Continue treating -ENODEV as success during
> device disconnect, consistent with the MDIO read path.
> 
> Fixes: 05b35e7eb9a1 ("smsc95xx: add phylib support")
> Signed-off-by: Artem Novak <artem122novak@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114050954.275495-1-artem122novak@gmail.com?part=2

Re: [PATCH net 2/2] net: usb: smsc95xx: propagate MDIO write errors

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch makes smsc95xx_mdio_write() return an error, but
  only one of its two callers was updated.

--

net: usb: smsc95xx: propagate MDIO write errors

This commit changes smsc95xx_mdio_write() from void to int so it returns
register access errors, while still treating -ENODEV as success during
disconnect. smsc95xx_mdiobus_write() now passes that error back to the
MDIO bus instead of always returning 0.

> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index f85ee5f8a5394..52035c160f4b0 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -215,8 +215,8 @@ static int smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx)
>  	return ret;
>  }
>  
> -static void smsc95xx_mdio_write(struct usbnet *dev, int phy_id, int idx,
> -				int regval)
> +static int smsc95xx_mdio_write(struct usbnet *dev, int phy_id, int idx,
> +			       int regval)

[Severity: Medium]
smsc95xx_mdio_write() has a second caller, smsc95xx_enter_suspend1(),
and this patch doesn't change it. Both writes there still throw away the
return value:

drivers/net/usb/smsc95xx.c:smsc95xx_enter_suspend1() {
    ...
	if (pdata->features & FEATURE_PHY_NLP_CROSSOVER)
		smsc95xx_mdio_write(dev, phy_id, PHY_EDPD_CONFIG,
				    PHY_EDPD_CONFIG_DEFAULT);

	/* enable energy detect power-down mode */
	ret = smsc95xx_mdio_read(dev, phy_id, PHY_MODE_CTRL_STS);
	if (ret < 0)
		return ret;

	ret |= MODE_CTRL_STS_EDPWRDOWN_;

	smsc95xx_mdio_write(dev, phy_id, PHY_MODE_CTRL_STS, ret);
    ...
}

Every other access in that function returns on error, including the
smsc95xx_mdio_read() between the two writes.

What happens if one of these writes fails? That could be an -EIO from
smsc95xx_phy_wait_not_busy(), or a USB error other than -ENODEV from
smsc95xx_write_reg().

In that case the function still writes PM_CTRL with PM_CTL_SUS_MODE_1
and PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_. It then sets SUSPEND_SUSPEND1 in
pdata->suspend_flags and returns 0. The controller waits for an
energy-detect wake, but the PHY may not be in energy-detect power-down.

smsc95xx_suspend() gets 0 back from smsc95xx_enter_suspend1(), both from
the autosuspend path and from the WAKE_PHY path with the link down. So
the usbnet_resume() recovery at its done: label never runs.

Should smsc95xx_enter_suspend1() check these return values too, since
the subject says MDIO write errors are propagated?

Also, unlike smsc95xx_read_reg(), smsc95xx_write_reg() and
smsc95xx_phy_wait_not_busy() in this file, the new int-returning helper
isn't marked __must_check. Would adding it make the compiler flag
dropped results like these?

>  {
>  	u32 val, addr;
>  	int ret;
> @@ -255,6 +255,12 @@ static void smsc95xx_mdio_write(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;
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114050954.275495-1-artem122novak%40gmail.com

Re: [PATCH net 0/2] net: usb: smsc95xx: propagate MDIO errors

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account.

*IMPORTANT*: If you do repost (for other reasons) - please include
the relevant info in the cover letter or commit messages.
-- 
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net 0/2] net: usb: smsc95xx: propagate MDIO errors

[Artem Novak]

Hi,

The issue was discovered after a kernel panic on my Raspberry Pi server.

I investigated the failure by inspecting the BCM2835 SDHOST driver's
DMA preparation and PIO fallback paths.
This led me to identify that when DMA mapping or descriptor
preparation fails, the final scatterlist
entry remains shortened after space has been reserved for FIFO drain words.
The request then falls back to PIO with a modified scatterlist.

The patch restores the original scatterlist length and clears the
drain word count
before returning to the fallback path.

Thanks,
Artem

On Fri, Nov 13, 2026 at 11:12 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by LLM and not proven at runtime). If it was triggered
>    please include the symptoms, like the stack trace or kernel messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account.
>
> *IMPORTANT*: If you do repost (for other reasons) - please include
> the relevant info in the cover letter or commit messages.
> --
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.