Thread (3 messages) flat view 3 messages, 3 authors, 5d ago
COOLING5d

[PATCH bpf-next v6] xsk: Fix circular locking dependency between &net->xdp.lock, &xs->mutex, and netdev_lock_ops

From: Khawar Ahemad <hidden>
Date: 2026-09-15 08:10:34
Also in: bpf, lkml
Subsystem: networking [general], the rest, xdp sockets (af_xdp) · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds, Magnus Karlsson, Maciej Fijalkowski

syzbot reported a circular locking dependency involving &net->xdp.lock,
&port->pnodes_lock, netdev_lock_ops(), and &xs->mutex:

-> #3 (&net->xdp.lock):
       xsk_notifier+0x3d/0x2c0 net/xdp/xsk.c:2106
       ipvlan_device_event+0x310/0x4e0 drivers/net/ipvlan/ipvlan_main.c:834
       unregister_netdevice_many_notify+0x808/0x18b0 net/core/dev.c:12518

-> #2 (&port->pnodes_lock):
       ipvlan_device_event+0x85/0x4e0 drivers/net/ipvlan/ipvlan_main.c:795
       notifier_call_chain+0xb5/0x410 kernel/notifier.c:85

-> #1 (&dev_instance_lock_key / netdev_lock_ops):
       netdev_lock_ops include/net/netdev_lock.h:42 [inline]
       xsk_bind+0x331/0x11d0 net/xdp/xsk.c:1627

-> #0 (&xs->mutex):
       xsk_diag_fill net/xdp/xsk_diag.c:113 [inline]
       xsk_diag_dump+0x2e0/0x4e0 net/xdp/xsk_diag.c:166

The cycle exists through the following dependency chain:
1. xsk_bind() acquired netdev_lock_ops() while holding &xs->mutex (#1).
2. Device unregistration in ipvlan_device_event() acquired
   &port->pnodes_lock (#2) and called xsk_notifier(), which acquired
   &net->xdp.lock (#3).
3. Both xsk_diag_dump() and xsk_notifier() acquire &xs->mutex while
   holding &net->xdp.lock (#0).

Break the circular dependency at its source in xsk_bind() by acquiring
netdev_lock_ops() before &xs->mutex. To preserve the exact errno precedence
(-EBUSY vs -ENODEV) without requiring the mutex to read sxdp_ifindex, we
look up the net_device first. If the device exists, we lock netdev_lock_ops()
and then &xs->mutex, evaluating the socket state safely. The cleanup path
is adjusted to unlock in reverse order, ensuring device references are
correctly maintained or dropped.

This single lock reordering fully eliminates the &xs->mutex ->
netdev_lock_ops() edge, resolving the 4-lock cycle without introducing
concurrency regressions in the diagnostic dump or notifier paths.

Fixes: 975b11ae9077 ("xsk: add socket allocate, create and bind")
Reported-by: syzbot+aa48b5fe7bfda62d1682@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=aa48b5fe7bfda62d1682
Signed-off-by: Khawar Ahemad <redacted>
---
v5 -> v6:
- Abandon the two-phase xsk_notifier() and xsk_diag_dump() decoupling from
  v3-v5, which introduced a Use-After-Free during concurrent xsk_release(),
  and an O(N^2) list traversal complexity in xsk_diag_dump().
- Break the cycle at its root in xsk_bind() by acquiring netdev_lock_ops()
  before &xs->mutex, fully removing the &xs->mutex -> netdev_lock_ops edge.
- Look up the net_device before locking &xs->mutex to preserve the original
  -EBUSY vs -ENODEV errno precedence without any behavioural change.
- Link to v5: https://lore.kernel.org/bpf/20260902041257.58374-1-ahemadkhawar123@gmail.com/ (local)

v4 -> v5:
- Rebase cleanly on latest bpf-next master.
- Link to v4: https://lore.kernel.org/bpf/20260826174744.3394-1-ahemadkhawar123@gmail.com/ (local)

v3 -> v4:
- Rebase cleanly on latest bpf-next master to resolve merge conflict.
- Update commit message to accurately describe the full 4-lock dependency
  chain (&net->xdp.lock, &port->pnodes_lock, netdev_lock_ops, &xs->mutex).
- Link to v3: https://lore.kernel.org/bpf/20260826173019.2917-1-ahemadkhawar123@gmail.com/ (local)

v2 -> v3:
- Fix direct AB-BA lock inversion in xsk_notifier() by performing device
  queue sweeps via xsk_get_pool_from_qid() outside &net->xdp.lock.
- Eliminate &net->xdp.lock -> &xs->mutex in xsk_diag_dump() by taking a
  temporary socket reference and releasing the lock before xsk_diag_fill().
- Link to v2: https://lore.kernel.org/bpf/20260826162110.99879-1-ahemadkhawar123@gmail.com/ (local)

v1 -> v2:
- Avoid reordering locks in xsk_bind() to preserve errno precedence.
- Link to v1: https://lore.kernel.org/bpf/20260825152152.86092-1-ahemadkhawar123@gmail.com/ (local)

 net/xdp/xsk.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 7855ee09c4..0a4106e98b 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -1612,20 +1612,22 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
 		return -EINVAL;
 
 	rtnl_lock();
+
+	dev = dev_get_by_index(sock_net(sk), sxdp->sxdp_ifindex);
+	if (dev)
+		netdev_lock_ops(dev);
+
 	mutex_lock(&xs->mutex);
 	if (xs->state != XSK_READY) {
 		err = -EBUSY;
 		goto out_release;
 	}
 
-	dev = dev_get_by_index(sock_net(sk), sxdp->sxdp_ifindex);
 	if (!dev) {
 		err = -ENODEV;
 		goto out_release;
 	}
 
-	netdev_lock_ops(dev);
-
 	if (!xs->rx && !xs->tx) {
 		err = -EINVAL;
 		goto out_unlock;
@@ -1762,18 +1764,20 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
 	}
 
 out_unlock:
-	if (err) {
-		dev_put(dev);
-	} else {
+	if (!err) {
 		/* Matches smp_rmb() in bind() for shared umem
 		 * sockets, and xsk_is_bound().
 		 */
 		smp_wmb();
 		WRITE_ONCE(xs->state, XSK_BOUND);
 	}
-	netdev_unlock_ops(dev);
 out_release:
 	mutex_unlock(&xs->mutex);
+	if (dev) {
+		netdev_unlock_ops(dev);
+		if (err)
+			dev_put(dev);
+	}
 	rtnl_unlock();
 	return err;
 }
-- 
2.54.0 (Apple Git-157)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help