[PATCH bpf-next v2] xsk: Fix circular locking dependency in xsk_notifier

Khawar Ahemad posted 1 patch 1 month ago
net/xdp/xsk.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
[PATCH bpf-next v2] xsk: Fix circular locking dependency in xsk_notifier
Posted by Khawar Ahemad 1 month ago
syzbot reported a circular locking dependency involving &net->xdp.lock,
&xs->mutex, and netdev_lock_ops():

-> #2 (&net->xdp.lock):
       xsk_notifier
       unregister_netdevice_many_notify

-> #1 (&xs->mutex):
       xsk_diag_dump

-> #0 (netdev_lock_ops):
       xsk_bind

In xsk_notifier(), xp_clear_dev() was called while holding &xs->mutex.
Because xp_clear_dev() acquires netdev_lock_ops(netdev), this created a
nested dependency of &xs->mutex -> netdev_lock_ops. Combined with
xsk_diag_dump() (&net->xdp.lock -> &xs->mutex) and device unregistration
(netdev_lock_ops -> &net->xdp.lock), this formed a circular locking cycle.

xp_clear_dev() operates strictly on the buffer pool and net_device, and
does not require &xs->mutex once the socket is unbound by xsk_unbind_dev().
Both xsk_notifier() and deferred pool release are serialized by rtnl_lock.

Fix this by capturing the pool pointer under &xs->mutex and calling
xp_clear_dev(pool) after releasing &xs->mutex in xsk_notifier().

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 <ahemadkhawar123@gmail.com>
---
v1 -> v2:
- Resolve the circular locking dependency in xsk_notifier() instead of
  reordering locks in xsk_bind(), avoiding ABBA lock inversion with
  xp_clear_dev().
- Preserve user-space errno precedence in xsk_bind().
- Reference the correct Fixes commit 2495b430e382.
- Link to v1: https://lore.kernel.org/bpf/20260825152152.86092-1-ahemadkhawar123@gmail.com/

 net/xdp/xsk.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 7855ee09c4..c2f47182dc 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -2106,6 +2106,7 @@ static int xsk_notifier(struct notifier_block *this,
 		mutex_lock(&net->xdp.lock);
 		sk_for_each(sk, &net->xdp.list) {
 			struct xdp_sock *xs = xdp_sk(sk);
+			struct xsk_buff_pool *pool = NULL;
 
 			mutex_lock(&xs->mutex);
 			if (xs->dev == dev) {
@@ -2113,12 +2114,16 @@ static int xsk_notifier(struct notifier_block *this,
 				if (!sock_flag(sk, SOCK_DEAD))
 					sk_error_report(sk);
 
+				pool = xs->pool;
 				xsk_unbind_dev(xs);
-
-				/* Clear device references. */
-				xp_clear_dev(xs->pool);
 			}
 			mutex_unlock(&xs->mutex);
+
+			/* Clear device references outside xs->mutex to avoid
+			 * lock inversion with netdev_lock_ops().
+			 */
+			if (pool)
+				xp_clear_dev(pool);
 		}
 		mutex_unlock(&net->xdp.lock);
 		break;
-- 
2.54.0 (Apple Git-157)
Re: [PATCH bpf-next v2] xsk: Fix circular locking dependency in xsk_notifier
Posted by bot+bpf-ci@kernel.org 1 month ago
>     xsk: Fix circular locking dependency in xsk_notifier
>
>     syzbot reported a circular locking dependency involving &net->xdp.lock,
>     &xs->mutex, and netdev_lock_ops():
>
>     -> #2 (&net->xdp.lock):
>            xsk_notifier
>            unregister_netdevice_many_notify
>
>     -> #1 (&xs->mutex):
>            xsk_diag_dump
>
>     -> #0 (netdev_lock_ops):
>            xsk_bind

Does this lockdep chain match the syzbot report? Looking at the cited
report (message ID 6a8c9fd9.dbb3a75c.7844.000c.GAE@google.com), it shows
a four-lock cycle, not three:

    -> #3 (&net->xdp.lock)
           xsk_notifier <- ipvlan_device_event <-
           unregister_netdevice_many_notify <- rtnl_dellink
    -> #2 (&port->pnodes_lock)
           ipvlan_device_event
    -> #1 (&dev_instance_lock_key#23)
           netdev_lock_ops in xsk_bind+0x331 net/xdp/xsk.c:1627
    -> #0 (&xs->mutex)
           xsk_diag_fill net/xdp/xsk_diag.c:113 / xsk_diag_dump:166

    Chain exists of:
      &xs->mutex --> &port->pnodes_lock --> &net->xdp.lock

The report includes &port->pnodes_lock from drivers/net/ipvlan as a
load-bearing link in the chain, which this description omits entirely.

The lock numbering is also reversed. The report uses #3/#2/#1/#0 with
&xs->mutex as #0 (the lock being acquired that triggers the warning),
whereas the commit message renumbers them as #2/#1/#0 and places
netdev_lock_ops as #0 instead.

The causal analysis seems inconsistent too. The commit message states
"In xsk_notifier(), xp_clear_dev() was called while holding &xs->mutex...
this created a nested dependency of &xs->mutex -> netdev_lock_ops."

However, the report attributes the &xs->mutex -> netdev_lock edge to
xsk_bind(), not xsk_notifier(). The report's #1 entry shows:

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

Could the commit message be revised to accurately describe the lockdep
chain from the cited report, including all four locks and the correct
code paths that create each dependency?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32990621157