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: [email protected]
Signed-off-by: Lianjie Wang <[email protected]>
---
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
