netdev - Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't discard phy_start

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: <b1defb1e-fd87-d84b-3ea1-efaa461ccd30@gmail.com>
Date:   Mon, 8 Nov 2021 20:35:53 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Benedikt Spranger <b.spranger@...utronix.de>
Cc:     Andrew Lunn <andrew@...n.ch>, Bastian Germann <bage@...utronix.de>,
        "Russell King (Oracle)" <linux@...linux.org.uk>,
        davem@...emloft.net, netdev@...r.kernel.org,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't discard
 phy_start_aneg's return

On 08.11.2021 20:02, Benedikt Spranger wrote:
> On Mon, 8 Nov 2021 19:01:23 +0100
> Heiner Kallweit <hkallweit1@...il.com> wrote:
> 
>> If we would like to support PHY's that don't support all MDI modes
>> then supposedly this would require to add ETHTOOL_LINK_MODE bits for
>> the MDI modes. Then we could use the generic mechanism to check the
>> bits in the "supported" bitmap.
> 
> The things are even worse:
> The chip supports only auto-MDIX at Gigabit and force MDI and

The Gigabit part seems to be normal. Gigabit supports neither
forced mode nor forced MDI settings.

> auto-MDIX in 10/100 modes. No force MDIX at all.
> 
> A validation callback from phy_ethtool_ksettings_set() before
> restarting the PHY seems reasonable for me. Something like:
> 
> 	/* Verify the settings we care about. */
> 	if (autoneg != AUTONEG_ENABLE && autoneg != AUTONEG_DISABLE)
> 	        return -EINVAL;
> 
>         if (autoneg == AUTONEG_ENABLE && linkmode_empty(advertising))
> 	        return -EINVAL;
> 
>         if (autoneg == AUTONEG_DISABLE &&
> 	    ((speed != SPEED_1000 &&
> 	      speed != SPEED_100 &&
>               speed != SPEED_10) ||
>              (duplex != DUPLEX_HALF &&
>               duplex != DUPLEX_FULL)))
>                 return -EINVAL;
> 
> 	if (phydev->validate_cmd && phydev->validate_cmd(cmd))
> 		return -EINVAL;
> 
> Thanks
>     Benedikt Spranger
>