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: <20211108160653.3d6127df@mitra>
Date:   Mon, 8 Nov 2021 16:06:53 +0100
From:   Benedikt Spranger <b.spranger@...utronix.de>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     bage@...utronix.de, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>, davem@...emloft.net,
        netdev@...r.kernel.org
Subject: Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't
 discard phy_start_aneg's return

On Mon, 8 Nov 2021 14:25:48 +0000
"Russell King (Oracle)" <linux@...linux.org.uk> wrote:

> On Mon, Nov 08, 2021 at 03:18:34PM +0100, bage@...utronix.de wrote:
> > From: Bastian Germann <bage@...utronix.de>
> > 
> > Take the return of phy_start_aneg into account so that ethtool will
> > handle negotiation errors and not silently accept invalid input.
> 
> I don't think this description is accurate. If we get to call
> phy_start_aneg() with invalid input, then something has already
> gone wrong.
The MDI/MDIX/auto-MDIX settings are not checked before calling
phy_start_aneg(). If the PHY supports forcing MDI and auto-MDIX, but
not forcing MDIX _phy_start_aneg() returns a failure, which is silently
ignored.

> As Andrew has already explained, an error from this
> function means that something went wrong with PHY communication.
Or for MDI/MDIX setings, the PHY does not support a feature/setting.

> All validation should have happened prior to this function being
> called.
OK.
Just to be clear: The PHY driver should check the settings and return
an error before calling phy_ethtool_ksettings_set() ?

Thanks
    Bene