linux-kernel - Re: [PATCH v2] net: phy: Fix return value when !CONFIG

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0fd79f9e-fe6d-4c85-8dda-7f64e917129c@gmail.com>
Date: Sun, 13 Apr 2025 15:50:03 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: hhtracer@...il.com, andrew@...n.ch, linux@...linux.org.uk
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 huhai <huhai@...inos.cn>
Subject: Re: [PATCH v2] net: phy: Fix return value when !CONFIG_PHYLIB

On 13.04.2025 15:37, hhtracer@...il.com wrote:
> From: huhai <huhai@...inos.cn>
> 
> 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.
> 
Is there actually any call site which doesn't select PHYLIB directly or
indirectly? When briefly checking I didn't find one.
So my question would be rather: Do we need/want stubs for the following
functions at all?

fwnode_get_phy_id
fwnode_mdio_find_device
fwnode_phy_find_device
device_phy_find_device
fwnode_get_phy_node
get_phy_device
phy_device_register


And a formal remark:
Your v2 has no change log, and please allow 24h before sending a new version.

> Returning ERR_PTR(-ENXIO) is the correct and consistent way to indicate
> that PHY support is not available, and it avoids such issues.
> 
> Signed-off-by: huhai <huhai@...inos.cn>
> ---
>  include/linux/phy.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index a2bfae80c449..be299c572d73 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1787,13 +1787,13 @@ static inline struct phy_device *device_phy_find_device(struct device *dev)
>  static inline
>  struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENXIO);
>  }
>  
>  static inline
>  struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENXIO);
>  }
>  
>  static inline int phy_device_register(struct phy_device *phy)