netdev - Re: [RFC PATCH v2 net-next 3/7] net: ethtool: record custom RSS contexts in the IDR

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: <bf7ba840-e876-eecb-8772-faa6aed6e7c7@gmail.com>
Date: Fri, 9 Jun 2023 21:01:33 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, pabeni@...hat.com,
 edumazet@...gle.com, netdev@...r.kernel.org, habetsm.xilinx@...il.com,
 sudheer.mogilappagari@...el.com
Subject: Re: [RFC PATCH v2 net-next 3/7] net: ethtool: record custom RSS
 contexts in the IDR

On 13/04/2023 02:49, Jakub Kicinski wrote:
> On Tue, 11 Apr 2023 19:26:11 +0100 edward.cree@....com wrote:
>>  	if (rxfh.rss_context)
>>  		ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
>>  					    &rxfh.rss_context, delete);
>> @@ -1350,6 +1377,40 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>>  		else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
>>  			dev->priv_flags |= IFF_RXFH_CONFIGURED;
>>  	}
> 
> This is probably transient but I think we're potentially leaking @ctx
> in a goto out hiding inside the context here, and...
> 
>> +	/* Update rss_ctx tracking */
>> +	if (create) {
>> +		/* Ideally this should happen before calling the driver,
>> +		 * so that we can fail more cleanly; but we don't have the
>> +		 * context ID until the driver picks it, so we have to
>> +		 * wait until after.
>> +		 */
>> +		if (WARN_ON(idr_find(&dev->ethtool->rss_ctx, rxfh.rss_context)))
>> +			/* context ID reused, our tracking is screwed */
>> +			goto out;
> 
> here.

Wasn't entirely transient.  Fixed for v3.