netdev - Re: [net-next,v3,03/11] net: phy: phy_link_topology: Track ports in phy_link

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20260203180250.2175436-1-horms@kernel.org>
Date: Tue,  3 Feb 2026 18:02:50 +0000
From: Simon Horman <horms@...nel.org>
To: maxime.chevallier@...tlin.com
Cc: Simon Horman <horms@...nel.org>,
	hkallweit1@...il.com,
	nicveronese@...il.com,
	f.fainelli@...il.com,
	pabeni@...hat.com,
	edumazet@...gle.com,
	dimitri.fedrau@...bherr.com,
	thomas.petazzoni@...tlin.com,
	kabel@...nel.org,
	kuba@...nel.org,
	vladimir.oltean@....com,
	andrew@...n.ch,
	davem@...emloft.net,
	romain.gantois@...tlin.com,
	linux-kernel@...r.kernel.org,
	o.rempel@...gutronix.de,
	linux@...linux.org.uk,
	kory.maincent@...tlin.com,
	mwojtas@...omium.org,
	christophe.leroy@...roup.eu,
	herve.codina@...tlin.com,
	netdev@...r.kernel.org,
	daniel@...rotopia.org
Subject: Re: [net-next,v3,03/11] net: phy: phy_link_topology: Track ports in phy_link_topology

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: phy: phy_link_topology: Track ports in phy_link_topology

This commit adds port tracking to phy_link_topology, keeping track of all
ports connected to a netdev. Ports are added when a PHY is attached and
removed when it is detached.

> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 7dc2ff10c74a..456f1144e83e 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c

[ ... ]

> +int phy_link_topo_add_port(struct net_device *dev, struct phy_port *port)
> +{
> +	struct phy_link_topology *topo;
> +	int ret;
> +
> +	topo = phy_link_topo_get_or_alloc(dev);
> +	if (IS_ERR(topo))
> +		return PTR_ERR(topo);
> +
> +	/* Attempt to re-use a previously allocated port_id */
> +	if (port->id)
> +		ret = xa_insert(&topo->ports, port->id, port, GFP_KERNEL);
> +	else
> +		ret = xa_alloc_cyclic(&topo->ports, &port->id, port,
> +				      xa_limit_32b, &topo->next_port_index,
> +				      GFP_KERNEL);
> +
> +	return ret;
> +}

[ ... ]

> @@ -89,8 +126,20 @@ int phy_link_topo_add_phy(struct net_device *dev,
>  	if (ret < 0)
>  		goto err;
> +
> +	/* Add all the PHY's ports to the topology */
> +	list_for_each_entry(port, &phy->ports, head) {
> +		ret = phy_link_topo_add_port(dev, port);
> +		if (ret)
> +			goto del_ports;
                   ^^^

xa_alloc_cyclic() returns 1 when ID allocation wraps around, which is still a
successful allocation. The check at line 126 above correctly uses `if (ret < 0)`
for the phyindex allocation:

    ret = xa_alloc_cyclic(&topo->phys, &phy->phyindex, pdn, ...);
    if (ret < 0)
        goto err;

Should this use `if (ret < 0)` as well for consistency? With the current code,
when the port ID wraps around, the PHY attachment will fail and trigger rollback
of all successfully added ports even though the allocation succeeded.