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

[sashiko-bot@kernel.org]

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