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

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

[Khawar Ahemad]

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch fails to resolve the reported 3-lock circular dependency and introduces a new direct AB-BA deadlock between `net->xdp.lock` and `netdev_lock_ops`.
--

commit c3270120d7443d72c4a80a4f3f94211cd288531b
Author: Khawar Ahemad <ahemadkhawar123@gmail.com>

xsk: Fix circular locking dependency in xsk_notifier

This commit attempts to fix a syzbot-reported 3-lock circular dependency
involving &net->xdp.lock, &xs->mutex, and netdev_lock_ops(). It does so by
deferring the xp_clear_dev() call until after &xs->mutex is released.

Link: https://lore.kernel.org/bpf/20260825152152.86092-1-ahemadkhawar123@gmail.com/
Link: https://syzkaller.appspot.com/bug?extid=aa48b5fe7bfda62d1682

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7855ee09c4b64..c2f47182dc489 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);

[Severity: High]
Does this actually resolve the circular locking dependency, or does it
introduce a direct AB-BA deadlock?

The commit message notes that device unregistration establishes a dependency
from netdev_lock_ops to &net->xdp.lock.

By moving xp_clear_dev() (which acquires netdev_lock_ops()) outside of
xs->mutex, it is still being called while holding &net->xdp.lock in
xsk_notifier().

Doesn't this create a direct &net->xdp.lock -> netdev_lock_ops dependency?

Combined with the device unregistration dependency (netdev_lock_ops ->
&net->xdp.lock), won't this form a new 2-lock AB-BA deadlock?

Furthermore, does this actually break the original 3-lock cycle?

The patch claims to remove the &xs->mutex -> netdev_lock_ops dependency to
break the cycle.  However, doesn't this dependency still exist in xsk_bind(),
which calls netdev_lock_ops() while holding xs->mutex?

If so, it appears the original 3-lock cycle (via xsk_diag_dump, xsk_bind,
and device unregistration) remains unbroken by this patch.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826162110.99879-1-ahemadkhawar123@gmail.com?part=1

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

>     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