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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a direct ABBA lock inversion between `xs->mutex` and `netdev_lock_ops(dev)` while failing to fix the original lockdep cycle.
--

--- Patch [1]: [PATCH bpf-next] xsk: Fix circular locking dependency in xsk_bind ---
commit d56fb43288cd7168ec871fc7dae61dd873e064c7
Author: Khawar Ahemad <ahemadkhawar123@gmail.com>

    xsk: Fix circular locking dependency in xsk_bind

This commit attempts to fix a lockdep splat in xsk_bind() by acquiring
netdev_lock_ops(dev) before xs->mutex. It aims to align with the global lock
hierarchy.

Link: https://syzkaller.appspot.com/bug?extid=aa48b5fe7bfda62d1682

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7855ee09c4b64..d2fbbeb7b6d77 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1612,19 +1612,18 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
>  		return -EINVAL;
>  
>  	rtnl_lock();
> -	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;
> +		goto out_rtnl_unlock;
>  	}
>  
>  	netdev_lock_ops(dev);
> +	mutex_lock(&xs->mutex);

[Severity: High]
Does this change introduce a direct ABBA lock inversion between xs->mutex and
netdev_lock_ops(dev)?

While xsk_bind() now acquires netdev_lock_ops(dev) before xs->mutex,
xsk_notifier() still acquires xs->mutex before calling xp_clear_dev():

net/xdp/xsk.c:xsk_notifier() {
    ...
    mutex_lock(&xs->mutex);
    if (xs->dev == dev) {
        ...
        xp_clear_dev(xs->pool);
    }
    mutex_unlock(&xs->mutex);
    ...
}

And xp_clear_dev() unconditionally acquires netdev_lock_ops(netdev):

net/xdp/xsk_buff_pool.c:xp_clear_dev() {
    ...
    netdev_lock_ops(netdev);
    xp_disable_drv_zc(pool);
    xsk_clear_pool_at_qid(pool->netdev, pool->queue_id);
    ...
}

Could this lead to a deadlock when xsk_bind() races with xsk_notifier()?

Since xsk_notifier() still acquires xs->mutex before netdev_lock_ops(), does
this mean the original lockdep cycle reported in the commit message is not
actually broken?

> +	if (xs->state != XSK_READY) {
> +		err = -EBUSY;
> +		goto out_unlock;
> +	}
>  
>  	if (!xs->rx && !xs->tx) {
>  		err = -EINVAL;

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