Commit 5464c3a0 authored by Ilya Maximets's avatar Ilya Maximets Committed by Daniel Borkmann
Browse files

xdp: fix potential deadlock on socket mutex 



There are 2 call chains:

  a) xsk_bind --> xdp_umem_assign_dev
  b) unregister_netdevice_queue --> xsk_notifier

with the following locking order:

  a) xs->mutex --> rtnl_lock
  b) rtnl_lock --> xdp.lock --> xs->mutex

Different order of taking 'xs->mutex' and 'rtnl_lock' could produce a
deadlock here. Fix that by moving the 'rtnl_lock' before 'xs->lock' in
the bind call chain (a).

Reported-by: default avatar <syzbot+bf64ec93de836d7f4c2c@syzkaller.appspotmail.com>
Fixes: 455302d1 ("xdp: fix hang while unregistering device bound to xdp socket")
Signed-off-by: default avatarIlya Maximets <i.maximets@samsung.com>
Acked-by: default avatarJonathan Lemon <jonathan.lemon@gmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent 67571640

net/xdp/xdp_umem.c

+6 −10
Original line number Diff line number Diff line
@@ -87,21 +87,20 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev, 
	struct netdev_bpf bpf;

	int err = 0;



	ASSERT_RTNL();



	force_zc = flags & XDP_ZEROCOPY;

	force_copy = flags & XDP_COPY;



	if (force_zc && force_copy)

		return -EINVAL;



	rtnl_lock();

	if (xdp_get_umem_from_qid(dev, queue_id)) {

		err = -EBUSY;

		goto out_rtnl_unlock;

	}

	if (xdp_get_umem_from_qid(dev, queue_id))

		return -EBUSY;



	err = xdp_reg_umem_at_qid(dev, umem, queue_id);

	if (err)

		goto out_rtnl_unlock;

		return err;



	umem->dev = dev;

	umem->queue_id = queue_id;
@@ -110,7 +109,7 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev, 


	if (force_copy)

		/* For copy-mode, we are done. */

		goto out_rtnl_unlock;

		return 0;



	if (!dev->netdev_ops->ndo_bpf ||

	    !dev->netdev_ops->ndo_xsk_async_xmit) {
@@ -125,7 +124,6 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev, 
	err = dev->netdev_ops->ndo_bpf(dev, &bpf);

	if (err)

		goto err_unreg_umem;

	rtnl_unlock();



	umem->zc = true;

	return 0;
@@ -135,8 +133,6 @@ err_unreg_umem: 
		err = 0; /* fallback to copy mode */

	if (err)

		xdp_clear_umem_at_qid(dev, queue_id);

out_rtnl_unlock:

	rtnl_unlock();

	return err;

}

net/xdp/xsk.c

+2 −0
Original line number Diff line number Diff line
@@ -430,6 +430,7 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len) 
	if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY))

		return -EINVAL;



	rtnl_lock();

	mutex_lock(&xs->mutex);

	if (xs->state != XSK_READY) {

		err = -EBUSY;
@@ -515,6 +516,7 @@ out_unlock: 
		xs->state = XSK_BOUND;

out_release:

	mutex_unlock(&xs->mutex);

	rtnl_unlock();

	return err;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE