netdev - Re: [PATCH net-next 1/8] net: avoid potential race between netdev_get_by_index

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: <Z_SMlQBO2rmtkJwC@LQ3V64L9R2>
Date: Mon, 7 Apr 2025 19:40:21 -0700
From: Joe Damato <jdamato@...tly.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,
	sdf@...ichev.me, hramamurthy@...gle.com, kuniyu@...zon.com
Subject: Re: [PATCH net-next 1/8] net: avoid potential race between
 netdev_get_by_index_lock() and netns switch

On Mon, Apr 07, 2025 at 12:01:10PM -0700, Jakub Kicinski wrote:
> netdev_get_by_index_lock() performs following steps:
> 
>   rcu_lock();
>   dev = lookup(netns, ifindex);
>   dev_get(dev);
>   rcu_unlock();
>   [... lock & validate the dev ...]
>   return dev
> 
> Validation right now only checks if the device is registered but since
> the lookup is netns-aware we must also protect against the device
> switching netns right after we dropped the RCU lock. Otherwise
> the caller in netns1 may get a pointer to a device which has just
> switched to netns2.
> 
> We can't hold the lock for the entire netns change process (because of
> the NETDEV_UNREGISTER notifier), and there's no existing marking to
> indicate that the netns is unlisted because of netns move, so add one.
> 
> AFAIU none of the existing netdev_get_by_index_lock() callers can
> suffer from this problem (NAPI code double checks the netns membership
> and other callers are either under rtnl_lock or not ns-sensitive),
> so this patch does not have to be treated as a fix.
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
>  include/linux/netdevice.h |  6 +++++-
>  net/core/dev.h            |  2 +-
>  net/core/dev.c            | 25 ++++++++++++++++++-------
>  3 files changed, 24 insertions(+), 9 deletions(-)

Reviewed-by: Joe Damato <jdamato@...tly.com>