[PATCH net v2] net: ethtool: phy: warn about a phydev with no link topology

[PATCH net v2] net: ethtool: phy: warn about a phydev with no link topology

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

phy_prepare_data() reads reply_data->dev->link_topo and dereferences it
without checking it:

	struct phy_link_topology *topo = reply_data->dev->link_topo;
	...
	pdn = xa_load(&topo->phys, phydev->phyindex);

ethnl_req_get_phydev() returns dev->phydev as-is when the request carries
no phy-index, without consulting the topology, so a driver that populates
net_device->phydev without going through phy_attach_direct() reaches the
xa_load() above with topo == NULL. The IBM EMAC driver did exactly that on
its fixed-link path; it is fixed separately by "net: ibm: emac: attach the
fixed-link PHY with of_phy_connect()".

This is the only place in net/ethtool/ that touches ->link_topo without
coping with a missing topology: phy_link_topo_empty() is "return
!dev->link_topo", phy_link_topo_get_phy() and phy_link_topo_get_port()
both return NULL when it is NULL, and common.c, port.c, tsinfo.c and
ethnl_perphy_dump_one_dev() all test dev->link_topo first. Before
commit 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to
generic phy dump"), ethnl_phy_parse_request() bailed out with "if (!topo)
return 0;" before reaching the xarray; that check was not carried over
into phy_prepare_data().

A netdev with a phydev but no topology is a driver bug and is worth being
told about, so do not fail silently: warn once and answer -EOPNOTSUPP,
which is what phy_prepare_data() already returns when the phydev cannot be
found in the topology. Compared with the current NULL dereference this
keeps the driver bug in the log without oopsing the box for somebody who
merely ran ethtool on an otherwise working interface, which is a rather
indirect way of pointing at a MAC driver. The dump side already returns
early when dev->link_topo is NULL, so the warning can only fire from the
doit path, at most once per boot.

Verified by compilation; there is no EMAC fixed-link device here to
trigger it.

Fixes: 9dd2ad5e92b9 ("net: ethtool: phy: Convert the PHY_GET command to generic phy dump")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
v2: reworked after Andrew Lunn's review.

 - Point the finger at the real bug: the IBM EMAC driver assigning
   net_device->phydev by hand on its fixed-link path. That is now fixed
   separately with "net: ibm: emac: attach the fixed-link PHY with
   of_phy_connect()".
 - Keep the driver bug visible instead of papering over it: v1 silently
   returned -EOPNOTSUPP, v2 warns once and then returns -EOPNOTSUPP.
   Happy to drop this patch entirely if the NULL dereference is preferred
   as the way to catch such drivers.
 - Dropped Cc: stable.

v1: 
 - Link: https://lore.kernel.org/all/20261111072146.3293821-1-xiaolinkui@126.com/
---
 net/ethtool/phy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
index 018b041..7e7e244 100644
--- a/net/ethtool/phy.c
+++ b/net/ethtool/phy.c
@@ -83,6 +83,9 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info,
 	if (IS_ERR_OR_NULL(phydev))
 		return -EOPNOTSUPP;
 
+	if (WARN_ON_ONCE(!topo))
+		return -EOPNOTSUPP;
+
 	pdn = xa_load(&topo->phys, phydev->phyindex);
 	if (!pdn)
 		return -EOPNOTSUPP;
-- 
2.25.1

Re: [PATCH net v2] net: ethtool: phy: warn about a phydev with no link topology

[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.

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. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

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 v2] net: ethtool: phy: warn about a phydev with no link topology

[Linkui Xiao]

Hi,

On 2026/11/12 16:02, 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.
> 
> 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. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.
Thanks for the reminder. The issue was found during an audit of
net/ethtool/ assisted by an LLM (Codex). The audit went through every
place that dereferences dev->link_topo, and phy_prepare_data() stood out
as the only one that does not cope with a missing topology. Following
that back led to the IBM EMAC driver assigning ndev->phydev by hand on
its fixed-link path, bypassing phy_attach_direct().

I verified the Fixes: commit (9dd2ad5e92b9) and both call paths (doit
and dump) by reading the code. The issue was not hit in production: no
in-tree DTS pairs "ibm,emac" with "fixed-link", so it is latent in
mainline and only out-of-tree users such as the Netgear WNDAP620 would
hit it.

As you suggested, I am not reposting the series for this. If a v3 is
needed for other reasons, I will fold this into the commit messages.

Thanks,
Linkui