netdev - Re: [PATCH net] net: ena: fix duplicate Autoneg setting in get_link

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: <6500d036-f6ef-47df-9158-529b2f376fae@lunn.ch>
Date: Thu, 11 Sep 2025 16:27:54 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Alok Tiwari <alok.a.tiwari@...cle.com>
Cc: shayagr@...zon.com, akiyano@...zon.com, saeedb@...zon.com,
	darinzon@...zon.com, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	horms@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: ena: fix duplicate Autoneg setting in
 get_link_ksettings

On Thu, Sep 11, 2025 at 04:37:20AM -0700, Alok Tiwari wrote:
> The ENA ethtool implementation mistakenly sets the Autoneg link mode
> twice in the 'supported' mask, leaving the 'advertising mask unset.

These are not masks. They are bitfields.

> Fix this by setting Autoneg in 'advertising' instead of duplicating
> it in 'supported'.
> 
> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
> Signed-off-by: Alok Tiwari <alok.a.tiwari@...cle.com>
> ---
>  drivers/net/ethernet/amazon/ena/ena_ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> index a81d3a7a3bb9..a6ef12c157ca 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
> @@ -471,7 +471,7 @@ static int ena_get_link_ksettings(struct net_device *netdev,
>  		ethtool_link_ksettings_add_link_mode(link_ksettings,
>  						     supported, Autoneg);
>  		ethtool_link_ksettings_add_link_mode(link_ksettings,
> -						     supported, Autoneg);
> +						     advertising, Autoneg);

While i agree the current code looks wrong, i'm not convinced your
change is correct.

What does ENA_ADMIN_GET_FEATURE_LINK_DESC_AUTONEG_MASK mean?

That the firmware support autoneg? If so, setting the bit in supported
makes sense. But does it mean it is actually enabled? If its not
enabled, you should not set it in advertising.

However, if we assume the firmware always supports autoneg, but
ENA_ADMIN_GET_FEATURE_LINK_DESC_AUTONEG_MASK indicates it is enabled,
supported should always have the bit set, and advertising should be
set based on this flag.

	Andrew