Commit 22cbc1ee authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

netdev: fix the locking for netdev notifications 



Kuniyuki reports that the assert for netdev lock fires when
there are netdev event listeners (otherwise we skip the netlink
event generation).

Correct the locking when coming from the notifier.

The NETDEV_XDP_FEAT_CHANGE notifier is already fully locked,
it's the documentation that's incorrect.

Fixes: 99e44f39 ("netdev: depend on netdev->lock for xdp features")
Reported-by: default avatarsyzkaller <syzkaller@googlegroups.com>
Reported-by: default avatarKuniyuki Iwashima <kuniyu@amazon.com>
Link: https://lore.kernel.org/20250410171019.62128-1-kuniyu@amazon.com


Acked-by: default avatarStanislav Fomichev <sdf@fomichev.me>
Link: https://patch.msgid.link/20250416030447.1077551-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent cfba1d1b

Documentation/networking/netdevices.rst

+3 −1
Original line number Diff line number Diff line
@@ -387,12 +387,14 @@ For device drivers that implement shaping or queue management APIs, 
some of the notifiers (``enum netdev_cmd``) are running under the netdev

instance lock.



The following netdev notifiers are always run under the instance lock:

* ``NETDEV_XDP_FEAT_CHANGE``



For devices with locked ops, currently only the following notifiers are

running under the lock:

* ``NETDEV_CHANGE``

* ``NETDEV_REGISTER``

* ``NETDEV_UP``

* ``NETDEV_XDP_FEAT_CHANGE``



The following notifiers are running without the lock:

* ``NETDEV_UNREGISTER``

include/linux/netdevice.h

+1 −1
Original line number Diff line number Diff line
@@ -2520,7 +2520,7 @@ struct net_device { 
	 *	@net_shaper_hierarchy, @reg_state, @threaded

	 *

	 * Double protects:

	 *	@up, @moving_ns, @nd_net, @xdp_flags

	 *	@up, @moving_ns, @nd_net, @xdp_features

	 *

	 * Double ops protects:

	 *	@real_num_rx_queues, @real_num_tx_queues

include/net/netdev_lock.h

+16 −0
Original line number Diff line number Diff line
@@ -48,6 +48,22 @@ static inline void netdev_unlock_ops(struct net_device *dev) 
		netdev_unlock(dev);

}



static inline void netdev_lock_ops_to_full(struct net_device *dev)

{

	if (netdev_need_ops_lock(dev))

		netdev_assert_locked(dev);

	else

		netdev_lock(dev);

}



static inline void netdev_unlock_full_to_ops(struct net_device *dev)

{

	if (netdev_need_ops_lock(dev))

		netdev_assert_locked(dev);

	else

		netdev_unlock(dev);

}



static inline void netdev_ops_assert_locked(const struct net_device *dev)

{

	if (netdev_need_ops_lock(dev))

net/core/lock_debug.c

+3 −1
Original line number Diff line number Diff line
@@ -18,10 +18,12 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event, 


	/* Keep enum and don't add default to trigger -Werror=switch */

	switch (cmd) {

	case NETDEV_XDP_FEAT_CHANGE:

		netdev_assert_locked(dev);

		fallthrough;

	case NETDEV_CHANGE:

	case NETDEV_REGISTER:

	case NETDEV_UP:

	case NETDEV_XDP_FEAT_CHANGE:

		netdev_ops_assert_locked(dev);

		fallthrough;

	case NETDEV_DOWN:

net/core/netdev-genl.c

+4 −0
Original line number Diff line number Diff line
@@ -963,10 +963,14 @@ static int netdev_genl_netdevice_event(struct notifier_block *nb, 


	switch (event) {

	case NETDEV_REGISTER:

		netdev_lock_ops_to_full(netdev);

		netdev_genl_dev_notify(netdev, NETDEV_CMD_DEV_ADD_NTF);

		netdev_unlock_full_to_ops(netdev);

		break;

	case NETDEV_UNREGISTER:

		netdev_lock(netdev);

		netdev_genl_dev_notify(netdev, NETDEV_CMD_DEV_DEL_NTF);

		netdev_unlock(netdev);

		break;

	case NETDEV_XDP_FEAT_CHANGE:

		netdev_genl_dev_notify(netdev, NETDEV_CMD_DEV_CHANGE_NTF);