netdev - Re: [PATCH net-next 3/9] net: phy: c45: don't accept disabled EEE modes in genphy_c45_ethtool_set

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] [day] [month] [year] [list]

Message-ID: <131dd9af-1fe2-4593-88c9-febd77b54ed1@gmail.com>
Date: Sat, 11 Jan 2025 18:52:45 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: "Russell King (Oracle)" <linux@...linux.org.uk>,
 Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 David Miller <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Simon Horman <horms@...nel.org>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 3/9] net: phy: c45: don't accept disabled EEE
 modes in genphy_c45_ethtool_set_eee

On 11.01.2025 18:31, Andrew Lunn wrote:
>>> I disagree with some of this. Userspace should expect:
>>>
>>> - read current settings
>>> - copy supported modes to advertised modes
>>> - write current settings
>>>
>>> to work. If it fails, then how does ethtool, or even the user, work out
>>> which link modes are actually supported or not.
>>>
>>> If we're introducing a failure on the "disabled" modes, then that is
>>> a user-breaking change, and we need to avoid that. The current code
>>> silently ignored the broken modes, your new code would error out on
>>> the above action - and that's a bug.
>>>
>> OK, then I think what we can/should do:
>> - filter out disabled EEE modes when populating data->supported in
>>   genphy_c45_ethtool_get_eee
>> - silently filter out disabled EEE modes from user space provided
>>   EEE advertisement in genphy_c45_ethtool_set_eee
> 
> Ideally, the kAPI should work just the same as normal advertised
> modes. The read API returns what can actually be used, and write API
> expects a subset of that.
> 
> But maybe we have too much history and cannot enforce the subset
> without regressions, we just silently ignore the extra modes?
> 
> It might be too much plumbing, but it would be nice to include an
> extack saying some modes have been ignored? I _think_ extack can be
> used without an error.
> 
Nice idea, this could be done as a follow-up. I'll have a look into this.

> 	Andrew
> 
Heiner
> 
> 
>