Re: [PATCH net] net: usb: lan78xx: avoid NULL PHY dereference in WoL setup

[netdev-bot+sashiko@kernel.org]

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] On a LAN7801 in fixed-link mode with no PHY, lan78xx_set_wol()
  (drivers/net/usb/lan78xx.c) now accepts WAKE_PHY and returns success,…

--

net: usb: lan78xx: avoid NULL PHY dereference in WoL setup

This commit adds a check in lan78xx_set_wol() so that
phy_ethtool_set_wol() is only called when netdev->phydev is non-NULL.
This avoids a NULL pointer dereference on LAN7801 fixed-link setups
that have no PHY attached.

> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 5655941f14783..0c19a84623523 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1804,8 +1804,8 @@ static int lan78xx_set_wol(struct net_device *netdev,
>  	ret = device_set_wakeup_enable(&dev->udev->dev, (bool)wol->wolopts);
>  	if (ret < 0)
>  		goto exit_pm_put;
> -
> -	ret = phy_ethtool_set_wol(netdev->phydev, wol);
> +	if (netdev->phydev)
> +		ret = phy_ethtool_set_wol(netdev->phydev, wol);

[Severity: Medium]
With no PHY attached, does this now accept WAKE_PHY and return success,
even though nothing can raise a PHY wake event?

In the fixed-link case, lan78xx_phy_init() returns early and leaves
netdev->phydev NULL:

	/* No PHY found, so set up a fixed link and return early.
	 * No need to configure PHY IRQ or attach to phylink.
	 */
	return 0;

In lan78xx_set_wol(), the wol->wolopts & ~WAKE_ALL check lets WAKE_PHY
through. Then pdata->wol = wol->wolopts is stored and
device_set_wakeup_enable() succeeds. With the new guard the PHY call is
skipped, so ret stays 0.

On the next system suspend, lan78xx_suspend() calls
lan78xx_set_suspend(dev, pdata->wol). That arms a PHY wake source that
has no PHY behind it:

lan78xx_set_suspend() {
    ...
	if (wol & WAKE_PHY) {
		temp_pmt_ctl |= PMT_CTL_PHY_WAKE_EN_;

		temp_pmt_ctl |= PMT_CTL_WOL_EN_;
    ...
}

lan78xx_get_wol() also keeps reporting wol->supported = WAKE_ALL, which
includes WAKE_PHY, so userspace is told the mode is supported and
active.

Before e110bc825897, this configuration used a registered fixed PHY
with the generic driver. That driver has no .set_wol, so
phy_ethtool_set_wol() returned -EOPNOTSUPP.

When netdev->phydev is NULL, should lan78xx_set_wol() reject WAKE_PHY,
and should lan78xx_get_wol() drop it from the supported mask?

>  
>  exit_pm_put:
>  	usb_autopm_put_interface(dev->intf);

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