With an XDP program attached, virtnet_xdp_set() adds nr_cpu_ids queue
pairs for XDP_TX and raises curr_queue_pairs and real_num_rx_queues to
match. virtnet_xsk_pool_enable() accepts any queue below
curr_queue_pairs, so an AF_XDP zero-copy socket can be bound to one of
them.
When the program is removed, the queue count goes down again while the
pool stays bound to the driver. When the socket is closed afterwards,
virtnet_xsk_pool_disable() rejects the queue with -EINVAL, and the core
frees the pool anyway:
WARNING: net/xdp/xsk_buff_pool.c:160 at xp_disable_drv_zc+0x1ee/0x260
BUG: KASAN: slab-use-after-free in xp_alloc_batch+0x930/0xb40
Read of size 8 at addr ffff8880028eb400 by task ksoftirqd/0/14
xp_alloc_batch+0x930/0xb40
try_fill_recv+0xb27/0x26e0
as rq->xsk_pool still points to it and the queue is refilled from it.
The device going away with XDP attached, e.g. on hot-unplug or driver
unbind, ends the same way without an explicit removal:
unregister_netdevice_many_notify() removes XDP before the xsk notifier
unbinds the socket, so the pool is disabled on a queue above
curr_queue_pairs there as well.
Removing XDP must not fail, so let virtnet_xsk_pool_disable() accept
any queue the driver has, below max_queue_pairs. The receive and send
queues above curr_queue_pairs still exist, so they are unbound as usual.
This was pointed out by the Sashiko review of v1 of this series, and
reproduced in QEMU by binding a socket to an XDP queue and then either
removing XDP and closing the socket, or unbinding the device from
virtio_net.
Fixes: 09d2b3182c8e ("virtio_net: xsk: bind/unbind xsk for rx")
Signed-off-by: Haishuang Yan <[email protected]>
---
drivers/net/virtio_net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4f5d8812f75b..f9d227060375 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -6030,7 +6030,8 @@ static int virtnet_xsk_pool_disable(struct net_device
*dev, u16 qid)
struct send_queue *sq;
int err;
- if (qid >= vi->curr_queue_pairs)
+ /* Removing XDP can leave the pool on a queue above curr_queue_pairs */
+ if (qid >= vi->max_queue_pairs)
return -EINVAL;
sq = &vi->sq[qid];
--
2.43.0
