Re: [PATCH net] virtio_net: reject stats replies for queues outside the request

[Paolo Abeni <pabeni@redhat.com>]

On 10/22/26 20:26, Lianjie Wang wrote:
> __virtnet_get_hw_stats() passes the vq_index of each record in the
> device's statistics reply to virtnet_fill_stats() without checking it,
> so the device decides where the driver writes the counters.
> 
> For ethtool -S, virtnet_fill_stats() uses vq_index as an offset into the
> output array, which the ethtool core sized for curr_queue_pairs. A
> record with the vq_index of a higher queue makes the driver write past
> the end of the array.
> 
> For the per-queue netlink stats, the queue type also comes from
> vq_index. A receive record in reply to a request for a transmit queue
> makes the driver fill the struct netdev_queue_stats_tx on the stack as
> if it were a struct netdev_queue_stats_rx. If the device offers
> VIRTIO_NET_STATS_TYPE_RX_SPEED, this writes one counter to the eight
> bytes after the tx struct.
> 
> Fix this by rejecting the reply when the vq_index of a record lies
> outside the range of queues the request asked about.
> 
> Claude Opus 5.5 was used to review the driver code and draft this patch.
> To reproduce, the virtio-net device in QEMU was modified to answer an
> ethtool -S request on a one-pair device with a record for a receive
> queue the device does not have. On an unmodified kernel, KASAN then
> reports a write past the ethtool -S output array. With this patch, the
> driver rejects the same reply with "Failed to get hw stats.", a
> well-formed reply fills the statistics as before, and the
> drivers/net/stats.py selftest passes.
> 
> Fixes: 941168f8b40e ("virtio_net: support device stats")
> Fixes: d888f04c09bb ("virtio-net: support queue stat")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lianjie Wang <karin0.zst@gmail.com>
> ---
>   drivers/net/virtio_net.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index daab43ac92ce4..0473fa9947858 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4955,7 +4955,8 @@ static void virtnet_fill_stats(struct virtnet_info *vi, u32 qid,
>   static int __virtnet_get_hw_stats(struct virtnet_info *vi,
>   				  struct virtnet_stats_ctx *ctx,
>   				  struct virtio_net_ctrl_queue_stats *req,
> -				  int req_size, void *reply, int res_size)
> +				  int req_size, void *reply, int res_size,
> +				  int first_vq, int last_vq, bool enable_cvq)
>   {
>   	struct virtio_net_stats_reply_hdr *hdr;
>   	struct scatterlist sgs_in, sgs_out;
> @@ -4976,6 +4977,10 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
>   	for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
>   		hdr = p;
>   		qid = le16_to_cpu(hdr->vq_index);
> +		if ((qid < first_vq || qid > last_vq) &&
> +		    !(enable_cvq && qid == vi->max_queue_pairs * 2))
> +			return -EINVAL;

I think the above check could be simplified to:
		if (qid < first_vq || qid > last_vq)
			return -EINVAL;

squashing the following in virtnet_get_hw_stats():

-       if (enable_cvq)
+       if (enable_cvq) {
                 virtnet_make_stat_req(vi, ctx, req, vi->max_queue_pairs * 2, &j);
+               last_vq++;
+       }

(likely with some meaningful comment).

/P