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: <20250401080036.5aad536b@kernel.org>
Date: Tue, 1 Apr 2025 08:00:36 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Pavel Begunkov <asml.silence@...il.com>
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 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.