xp_clear_dev() calls the driver to unbind an AF_XDP pool, then frees the pool. virtnet_xsk_pool_disable() currently rejects qid >= curr_queue_pairs, so if XDP detach (or another path) has already shrunk curr_queue_pairs below a still-bound qid, disable returns -EINVAL. The core only WARNs, clears the netdev pool pointer, and destroys the pool, which leaks the driver's DMA mappings and xsk_buffs and leaves rq/sq->xsk_pool dangling for a use-after-free if those queues are brought back later. Use max_queue_pairs for the bounds check so cleanup can still run, null the freed pointers, and avoid refill on inactive queues when unbinding. Also unmap the shared TX header with sq->vq on the enable error path to match the mapping side (no functional change while rq/sq share a DMA device). Fixes: 09d2b3182c8e ("virtio_net: xsk: bind/unbind xsk for rx") Signed-off-by: Xiong Weimin --- v2: - rewrite as a real fix for disable-after-shrink (leak / UAF) - drop the previous "check queue index before use" reorder patch - fold sq->vq unmap symmetry here as NFC (no separate Fixes for that) drivers/net/virtio_net.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 3e2a587..d07ccef 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -5848,7 +5848,7 @@ static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queu rq->xsk_pool = pool; - virtnet_rx_resume(vi, rq, true); + virtnet_rx_resume(vi, rq, qindex < vi->curr_queue_pairs); if (pool) return 0; @@ -5959,10 +5959,11 @@ err_sq: err_rq: xsk_pool_dma_unmap(pool, 0); err_xsk_map: - virtqueue_unmap_single_attrs(rq->vq, hdr_dma, vi->hdr_len, + virtqueue_unmap_single_attrs(sq->vq, hdr_dma, vi->hdr_len, DMA_TO_DEVICE, 0); err_free_buffs: kvfree(rq->xsk_buffs); + rq->xsk_buffs = NULL; return err; } @@ -5974,7 +5975,12 @@ static int virtnet_xsk_pool_disable(struct net_device *dev, u16 qid) struct send_queue *sq; int err; - if (qid >= vi->curr_queue_pairs) + /* rq/sq are sized by max_queue_pairs. Allow cleanup even if + * curr_queue_pairs has shrunk below qid (e.g. after XDP detach), + * otherwise disable fails, leaks mappings/xsk_buffs, and leaves + * dangling rq/sq->xsk_pool pointers to a soon-to-be-freed pool. + */ + if (qid >= vi->max_queue_pairs) return -EINVAL; sq = &vi->sq[qid]; @@ -5985,11 +5991,17 @@ static int virtnet_xsk_pool_disable(struct net_device *dev, u16 qid) err = virtnet_rq_bind_xsk_pool(vi, rq, NULL); err |= virtnet_sq_bind_xsk_pool(vi, sq, NULL); - xsk_pool_dma_unmap(pool, 0); + if (pool) + xsk_pool_dma_unmap(pool, 0); + + if (sq->xsk_hdr_dma_addr) { + virtqueue_unmap_single_attrs(sq->vq, sq->xsk_hdr_dma_addr, + vi->hdr_len, DMA_TO_DEVICE, 0); + sq->xsk_hdr_dma_addr = 0; + } - virtqueue_unmap_single_attrs(sq->vq, sq->xsk_hdr_dma_addr, - vi->hdr_len, DMA_TO_DEVICE, 0); kvfree(rq->xsk_buffs); + rq->xsk_buffs = NULL; return err; } -- 2.43.0 virtnet_xdp_set() can lower curr_queue_pairs when an XDP program is detached. Unlike ethtool channel updates, that path does not check for AF_XDP zero-copy pools on the queues being dropped. A pool can remain bound on a qid that is no longer covered by curr_queue_pairs, which breaks later unbind and can leave stale rq/sq->xsk_pool pointers. Refuse the shrink with -EBUSY while any AF_XDP pool is still bound on a queue that would become inactive. Fixes: 09d2b3182c8e ("virtio_net: xsk: bind/unbind xsk for rx") Signed-off-by: Xiong Weimin --- v2: - new patch: block XDP-driven queue shrink while AF_XDP is bound - replaces the previous series' approach after review drivers/net/virtio_net.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index d07ccef..3bd220d 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -6064,6 +6064,21 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, if (!prog && !old_prog) return 0; + /* ethtool channel shrink is gated on xsk_get_pool_from_qid(), but + * XDP detach shrinks curr_queue_pairs here without that check. + * Refusing the shrink keeps AF_XDP queues active until the socket + * unbinds them. + */ + if (curr_qp + xdp_qp < vi->curr_queue_pairs) { + for (i = curr_qp + xdp_qp; i < vi->curr_queue_pairs; i++) { + if (vi->rq[i].xsk_pool || vi->sq[i].xsk_pool) { + NL_SET_ERR_MSG_MOD(extack, + "Cannot reduce queues while AF_XDP is bound"); + return -EBUSY; + } + } + } + if (prog) bpf_prog_add(prog, vi->max_queue_pairs - 1); -- 2.43.0