netdev - Re: [PATCH net] net: ethtool: Fix RSS setting

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: <4077dffd-beb8-4db8-b729-3c741ae159a4@intel.com>
Date: Thu, 11 Jul 2024 07:41:16 -0600
From: Ahmed Zaki <ahmed.zaki@...el.com>
To: Saeed Mahameed <saeed@...nel.org>, "David S. Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>
CC: Saeed Mahameed <saeedm@...dia.com>, <netdev@...r.kernel.org>, Tariq Toukan
	<tariqt@...dia.com>, Gal Pressman <gal@...dia.com>, Leon Romanovsky
	<leonro@...dia.com>, David Wei <dw@...idwei.uk>, Pavel Begunkov
	<asml.silence@...il.com>
Subject: Re: [PATCH net] net: ethtool: Fix RSS setting



On 2024-07-10 4:55 p.m., Saeed Mahameed wrote:
> From: Saeed Mahameed <saeedm@...dia.com>
> 
> When user submits a rxfh set command without touching XFRM_SYM_XOR,
> rxfh.input_xfrm is set to RXH_XFRM_NO_CHANGE, which is equal to 0xff.
> 
> Testing if (rxfh.input_xfrm & RXH_XFRM_SYM_XOR &&
> 	    !ops->cap_rss_sym_xor_supported)
> 		return -EOPNOTSUPP;
> 
> Will always be true on devices that don't set cap_rss_sym_xor_supported,
> since rxfh.input_xfrm & RXH_XFRM_SYM_XOR is always true, if input_xfrm
> was not set, i.e RXH_XFRM_NO_CHANGE=0xff, which will result in failure
> of any command that doesn't require any change of XFRM, e.g RSS context
> or hash function changes.
> 
> To avoid this breakage, test if rxfh.input_xfrm != RXH_XFRM_NO_CHANGE
> before testing other conditions.
> 
> Fixes: 13e59344fb9d ("net: ethtool: add support for symmetric-xor RSS hash")
> CC: Ahmed Zaki <ahmed.zaki@...el.com>
> CC: David Wei <dw@...idwei.uk>,
> CC: Pavel Begunkov <asml.silence@...il.com>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> ---
>   net/ethtool/ioctl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index e645d751a5e8..223dcd25d88a 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -1306,7 +1306,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>   	if (rxfh.input_xfrm && rxfh.input_xfrm != RXH_XFRM_SYM_XOR &&
>   	    rxfh.input_xfrm != RXH_XFRM_NO_CHANGE)
>   		return -EINVAL;
> -	if ((rxfh.input_xfrm & RXH_XFRM_SYM_XOR) &&
> +	if (rxfh.input_xfrm != RXH_XFRM_NO_CHANGE &&
> +	    (rxfh.input_xfrm & RXH_XFRM_SYM_XOR) &&
>   	    !ops->cap_rss_sym_xor_supported)
>   		return -EOPNOTSUPP;
>   

LGTM, thank you for catching this.

Reviewed-by: Ahmed Zaki <ahmed.zaki@...el.com>