Commit a92c3228 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'net-fix-race-of-rtnl_net_lock-dev_net-dev' 

Kuniyuki Iwashima says:

====================
net: Fix race of rtnl_net_lock(dev_net(dev)).

Yael Chemla reported that commit 7fb10733 ("net: Hold rtnl_net_lock()
in (un)?register_netdevice_notifier_dev_net().") started to trigger KASAN's
use-after-free splat.

The problem is that dev_net(dev) fetched before rtnl_net_lock() might be
different after rtnl_net_lock().

The patch 2 fixes the issue by checking dev_net(dev) after rtnl_net_lock(),
and the patch 3 fixes the same potential issue that would emerge once RTNL
is removed.

v4: https://lore.kernel.org/20250212064206.18159-1-kuniyu@amazon.com
v3: https://lore.kernel.org/20250211051217.12613-1-kuniyu@amazon.com
v2: https://lore.kernel.org/20250207044251.65421-1-kuniyu@amazon.com
v1: https://lore.kernel.org/20250130232435.43622-1-kuniyu@amazon.com
====================

Link: https://patch.msgid.link/20250217191129.19967-1-kuniyu@amazon.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents f6093c5e d4c6bfc8

include/net/net_namespace.h

+11 −0
Original line number Diff line number Diff line
@@ -297,6 +297,7 @@ static inline int check_net(const struct net *net) 
}



void net_drop_ns(void *);

void net_passive_dec(struct net *net);



#else
@@ -326,8 +327,18 @@ static inline int check_net(const struct net *net) 
}



#define net_drop_ns NULL



static inline void net_passive_dec(struct net *net)

{

	refcount_dec(&net->passive);

}

#endif



static inline void net_passive_inc(struct net *net)

{

	refcount_inc(&net->passive);

}



/* Returns true if the netns initialization is completed successfully */

static inline bool net_initialized(const struct net *net)

{

net/core/dev.c

+46 −8
Original line number Diff line number Diff line
@@ -2070,6 +2070,42 @@ static void __move_netdevice_notifier_net(struct net *src_net, 
	__register_netdevice_notifier_net(dst_net, nb, true);

}



static void rtnl_net_dev_lock(struct net_device *dev)

{

	bool again;



	do {

		struct net *net;



		again = false;



		/* netns might be being dismantled. */

		rcu_read_lock();

		net = dev_net_rcu(dev);

		net_passive_inc(net);

		rcu_read_unlock();



		rtnl_net_lock(net);



#ifdef CONFIG_NET_NS

		/* dev might have been moved to another netns. */

		if (!net_eq(net, rcu_access_pointer(dev->nd_net.net))) {

			rtnl_net_unlock(net);

			net_passive_dec(net);

			again = true;

		}

#endif

	} while (again);

}



static void rtnl_net_dev_unlock(struct net_device *dev)

{

	struct net *net = dev_net(dev);



	rtnl_net_unlock(net);

	net_passive_dec(net);

}



int register_netdevice_notifier_dev_net(struct net_device *dev,

					struct notifier_block *nb,

					struct netdev_net_notifier *nn)
@@ -2077,6 +2113,11 @@ int register_netdevice_notifier_dev_net(struct net_device *dev, 
	struct net *net = dev_net(dev);

	int err;



	/* rtnl_net_lock() assumes dev is not yet published by

	 * register_netdevice().

	 */

	DEBUG_NET_WARN_ON_ONCE(!list_empty(&dev->dev_list));



	rtnl_net_lock(net);

	err = __register_netdevice_notifier_net(net, nb, false);

	if (!err) {
@@ -2093,13 +2134,12 @@ int unregister_netdevice_notifier_dev_net(struct net_device *dev, 
					  struct notifier_block *nb,

					  struct netdev_net_notifier *nn)

{

	struct net *net = dev_net(dev);

	int err;



	rtnl_net_lock(net);

	rtnl_net_dev_lock(dev);

	list_del(&nn->list);

	err = __unregister_netdevice_notifier_net(net, nb);

	rtnl_net_unlock(net);

	err = __unregister_netdevice_notifier_net(dev_net(dev), nb);

	rtnl_net_dev_unlock(dev);



	return err;

}
@@ -11880,11 +11920,9 @@ EXPORT_SYMBOL(unregister_netdevice_many); 
 */

void unregister_netdev(struct net_device *dev)

{

	struct net *net = dev_net(dev);



	rtnl_net_lock(net);

	rtnl_net_dev_lock(dev);

	unregister_netdevice(dev);

	rtnl_net_unlock(net);

	rtnl_net_dev_unlock(dev);

}

EXPORT_SYMBOL(unregister_netdev);

net/core/net_namespace.c

+4 −4
Original line number Diff line number Diff line
@@ -464,7 +464,7 @@ static void net_complete_free(void) 


}



static void net_free(struct net *net)

void net_passive_dec(struct net *net)

{

	if (refcount_dec_and_test(&net->passive)) {

		kfree(rcu_access_pointer(net->gen));
@@ -482,7 +482,7 @@ void net_drop_ns(void *p) 
	struct net *net = (struct net *)p;



	if (net)

		net_free(net);

		net_passive_dec(net);

}



struct net *copy_net_ns(unsigned long flags,
@@ -523,7 +523,7 @@ struct net *copy_net_ns(unsigned long flags, 
		key_remove_domain(net->key_domain);

#endif

		put_user_ns(user_ns);

		net_free(net);

		net_passive_dec(net);

dec_ucounts:

		dec_net_namespaces(ucounts);

		return ERR_PTR(rv);
@@ -672,7 +672,7 @@ static void cleanup_net(struct work_struct *work) 
		key_remove_domain(net->key_domain);

#endif

		put_user_ns(net->user_ns);

		net_free(net);

		net_passive_dec(net);

	}

	cleanup_net_task = NULL;

}