netdev - Re: [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for get

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]

Date: Mon, 24 Jun 2024 14:31:32 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Simon Horman <horms@...nel.org>, edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
 edumazet@...gle.com, pabeni@...hat.com, netdev@...r.kernel.org,
 habetsm.xilinx@...il.com, sudheer.mogilappagari@...el.com,
 jdamato@...tly.com, mw@...ihalf.com, linux@...linux.org.uk,
 sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com,
 hkelam@...vell.com, saeedm@...dia.com, leon@...nel.org,
 jacob.e.keller@...el.com, andrew@...n.ch, ahmed.zaki@...el.com
Subject: Re: [PATCH v6 net-next 8/9] net: ethtool: use the tracking array for
 get_rxfh on custom RSS contexts

On 20/06/2024 20:42, Simon Horman wrote:
> On Thu, Jun 20, 2024 at 06:47:11AM +0100, edward.cree@....com wrote:
>> From: Edward Cree <ecree.xilinx@...il.com>
>>
>> On 'ethtool -x' with rss_context != 0, instead of calling the driver to
>>  read the RSS settings for the context, just get the settings from the
>>  rss_ctx xarray, and return them to the user with no driver involvement.
>>
>> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
>> ---
>>  net/ethtool/ioctl.c | 25 ++++++++++++++++++++-----
>>  1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
>> index 9d2d677770db..ac562ee3662e 100644
>> --- a/net/ethtool/ioctl.c
>> +++ b/net/ethtool/ioctl.c
>> @@ -1199,6 +1199,7 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>>  	const struct ethtool_ops *ops = dev->ethtool_ops;
>>  	struct ethtool_rxfh_param rxfh_dev = {};
>>  	u32 user_indir_size, user_key_size;
>> +	struct ethtool_rxfh_context *ctx;
>>  	struct ethtool_rxfh rxfh;
>>  	u32 indir_bytes;
>>  	u8 *rss_config;
>> @@ -1246,11 +1247,25 @@ static noinline_for_stack int ethtool_get_rxfh(struct net_device *dev,
>>  	if (user_key_size)
>>  		rxfh_dev.key = rss_config + indir_bytes;
>>  
>> -	rxfh_dev.rss_context = rxfh.rss_context;
>> -
>> -	ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> -	if (ret)
>> -		goto out;
>> +	if (rxfh.rss_context) {
>> +		ctx = xa_load(&dev->ethtool->rss_ctx, rxfh.rss_context);
>> +		if (!ctx) {
>> +			ret = -ENOENT;
>> +			goto out;
>> +		}
>> +		if (rxfh_dev.indir)
>> +			memcpy(rxfh_dev.indir, ethtool_rxfh_context_indir(ctx),
>> +			       indir_bytes);
>> +		if (rxfh_dev.key)
>> +			memcpy(rxfh_dev.key, ethtool_rxfh_context_key(ctx),
>> +			       user_key_size);
>> +		rxfh_dev.hfunc = ctx->hfunc;
>> +		rxfh_dev.input_xfrm = ctx->input_xfrm;
> 
> Hi Edward,
> 
> The last line of this function is:
> 
> 	return ret;
> 
> With this patch applied, Smatch complains that ret may be used there
> when unintialised.
> 
> I think that occurs when the code reaches the line where this
> commentary has been placed in this email.

You (and Smatch) are quite right.  Fixed for v7.
-ed

> 
>> +	} else {
>> +		ret = dev->ethtool_ops->get_rxfh(dev, &rxfh_dev);
>> +		if (ret)
>> +			goto out;
>> +	}
>>  
>>  	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, hfunc),
>>  			 &rxfh_dev.hfunc, sizeof(rxfh.hfunc))) {
>>