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

Message-ID: <be4d2a28-3618-451f-ab08-432489360410@lunn.ch>
Date: Sat, 11 Jan 2025 18:31:04 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Heiner Kallweit <hkallweit1@...il.com>
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

> > 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.

	Andrew