[PATCH net v2 1/2] virtio_net: fix AF_XDP pool disable after queue shrink

[Xiong Weimin]

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 <xiongwei...@kylinos.cn>
---
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