netdev - Re: [PATCH net-next v6 2/7] net: phy: Support speed selection for PHY loopback

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: <57e02f9b-ff5e-44fa-b2fa-cbd7dd93408a@lunn.ch>
Date: Wed, 12 Feb 2025 02:48:42 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Gerhard Engleder <gerhard@...leder-embedded.com>
Cc: hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v6 2/7] net: phy: Support speed selection for
 PHY loopback

> +int phy_loopback(struct phy_device *phydev, bool enable, int speed)
> +{
> +	bool link_up = false;
> +	int ret = 0;
> +
> +	if (!phydev->drv)
> +		return -EIO;
> +
> +	mutex_lock(&phydev->lock);
> +
> +	if (enable && phydev->loopback_enabled) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (!enable && !phydev->loopback_enabled) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (enable) {
> +		/*
> +		 * Link up is signaled with a defined speed. If speed changes,
> +		 * then first link down and after that link up needs to be
> +		 * signaled.
> +		 */
> +		if (phydev->link && phydev->state == PHY_RUNNING) {
> +			/* link is up and signaled */
> +			if (speed && phydev->speed != speed) {
> +				/* signal link down and up for new speed */
> +				phydev->link = false;
> +				phydev->state = PHY_NOLINK;
> +				phy_link_down(phydev);

If you set the link down here...

> +
> +				link_up = true;
> +			}
> +		} else {
> +			/* link is not signaled */
> +			if (speed) {
> +				/* signal link up for new speed */
> +				link_up = true;
> +			}
> +		}
> +	}
> +
> +	if (phydev->drv->set_loopback)
> +		ret = phydev->drv->set_loopback(phydev, enable, speed);
> +	else
> +		ret = genphy_loopback(phydev, enable, speed);
> +
> +	if (ret)
> +		goto out;

and this fails, you leave the link down. You should make an attempt to
restore the link to the old state before returning the error.

	Andrew