netdev - Re: [PATCH net] net: avoid race between device unregistration and set

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: <173685130626.5893.2031391323809340270@kwain>
Date: Tue, 14 Jan 2025 11:41:46 +0100
From: Antoine Tenart <atenart@...nel.org>
To: Edward Cree <ecree.xilinx@...il.com>, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net] net: avoid race between device unregistration and set_channels

Quoting Edward Cree (2025-01-14 11:09:07)
> On 13/01/2025 16:18, Antoine Tenart wrote:
> > This is because unregister_netdevice_many_notify might run before
> > set_channels (both are under rtnl). When that happens, the rss lock is
> > being destroyed before being used again. Fix this by destroying the rss
> > lock in run_todo, outside an rtnl lock section and after all references
> > to net devices are gone.
> 
> The latter (refs gone) being the important part?  Doesn't seem
>  particularly relevant that we've dropped rtnl, this wording had me
>  confused for a little while as to why this closed the race.

Both are important as being outside the rtnl lock section means
set_channels had a chance to run, and waiting for refs that it
completed. Well, otherwise there would be a deadlock, so maybe this is a
superfluous detail after all.

> > Note that allowing to run set_channels after the rtnl section of the
> > unregistration path should be fine as it still runs before the
> > destructors (thanks to refcount). This patch does not change that.
> > 
> > Fixes: 87925151191b ("net: ethtool: add a mutex protecting RSS contexts")
> > Cc: Edward Cree <ecree.xilinx@...il.com>
> > Signed-off-by: Antoine Tenart <atenart@...nel.org>
> 
> Reviewed-by: Edward Cree <ecree.xilinx@...il.com>

Thanks!