Re: [PATCH v2] net: phy: Fix return value when !CONFIG_PHYLIB

[Russell King (Oracle)]

On Sun, Apr 13, 2025 at 09:37:09PM +0800, hhtracer@xxxxxxxxx wrote:
> Many call sites of get_phy_device() and fwnode_get_phy_node(), such as
> sfp_sm_probe_phy(), phylink_fwnode_phy_connect(), etc., rely on IS_ERR()
> to check for errors in the returned pointer.
> 
> Furthermore, the implementations of get_phy_device() and
> fwnode_get_phy_node() themselves use ERR_PTR() to return error codes.
> 
> Therefore, when CONFIG_PHYLIB is disabled, returning NULL is incorrect,
> as this would bypass IS_ERR() checks and may lead to NULL pointer
> dereference.
> 
> Returning ERR_PTR(-ENXIO) is the correct and consistent way to indicate
> that PHY support is not available, and it avoids such issues.

I wonder whether it's crossed one's mind that returning NULL may be
intentional to avoid erroring out at the callsites, and returning
an error may cause runtime failures.

You need to do way more investigation before posting a patch like this:

- Analyse each call site.
- Determine whether that call site can exist if PHYLIB is not built.
- Determine whether returning an error in that case instead of NULL
  may be detrimental, or at the very least list the call sites that
  would be affected by the change.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!