netdev - Re: [PATCH net 1/2] net: move mp dev config validation to __net_mp_open

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: <c53b8db8-fee0-4c50-8253-fd9ce6e5be0c@gmail.com>
Date: Wed, 2 Apr 2025 13:22:30 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 ap420073@...il.com, almasrymina@...gle.com, dw@...idwei.uk, sdf@...ichev.me
Subject: Re: [PATCH net 1/2] net: move mp dev config validation to
 __net_mp_open_rxq()

On 4/1/25 16:00, Jakub Kicinski wrote:
> On Tue, 1 Apr 2025 12:37:34 +0100 Pavel Begunkov wrote:
>>> -	err = xa_alloc(&binding->bound_rxqs, &xa_idx, rxq, xa_limit_32b,
>>> -		       GFP_KERNEL);
>>> +	err = __net_mp_open_rxq(dev, rxq_idx, &mp_params, extack);
>>>    	if (err)
>>>    		return err;
>>
>> Was reversing the order b/w open and xa_alloc intentional?
>> It didn't need __net_mp_close_rxq() before, which is a good thing
>> considering the error handling in __net_mp_close_rxq is a bit
>> flaky (i.e. the WARN_ON at the end).
> 
> Should have mentioned.. yes, intentional, otherwise we'd either have to
> insert a potentially invalid rxq pointer into the xarray or duplicate
> the rxq bounds check. Inserting invalid pointer and deleting it immediately
> should be okay, since readers take the instance lock, but felt a little
> dirty. In practice xa_alloc() failures should be extremely rare here so
> I went with the reorder.

I see, sgtm

-- 
Pavel Begunkov