__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 can make 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. The control vq is
checked apart from that range, since the vqs of the queue pairs beyond
curr_queue_pairs lie between that range and the control vq.
Claude Opus 5.5 was used to audit the driver and draft this patch. To
reproduce, the virtio-net device in QEMU, which has one queue pair, was
modified to answer an ethtool -S request with a record for a nonexistent
receive queue. On an unmodified kernel, KASAN reports a write past the
ethtool -S output array:
BUG: KASAN: vmalloc-out-of-bounds in virtnet_fill_stats+0xab1/0xec0
Write of size 8 at addr ffffc90000066140 by task ethtool_stats/159
Call Trace:
virtnet_fill_stats+0xab1/0xec0
virtnet_get_hw_stats+0x5d6/0x7c0
virtnet_get_ethtool_stats+0x11b/0x920
ethtool_get_stats+0x150/0x1f0
dev_ethtool+0xaaa/0x1be0
The buggy address belongs to a 1-page vmalloc region starting at
0xffffc90000066000 allocated at ethtool_get_stats+0x120/0x1f0
With this patch, the driver rejects the same reply with "Failed to get
hw stats.". When ethtool -L leaves queue pairs unused, the driver also
rejects a record for the first unused receive queue, while a well-formed
reply fills the statistics as before, those of the control vq included.
The drivers/net/stats.py selftest passes on a device that reports
receive and transmit statistics.
Fixes: 941168f8b40e ("virtio_net: support device stats")
Fixes: d888f04c09bb ("virtio-net: support queue stat")
Cc: [email protected]
Signed-off-by: Lianjie Wang <[email protected]>
---
v2:
- Add a comment and a sentence on why the control vq is checked apart
from the range of the other queues.
- Add the KASAN report and a test with unused queue pairs.
v1: https://lore.kernel.org/all/[email protected]/
drivers/net/virtio_net.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index dd2c41ef6fbe7..db56f984bb86a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -4973,7 +4973,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;
@@ -4994,6 +4995,13 @@ 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);
+ /* The vqs of the queue pairs beyond curr_queue_pairs lie
+ * between last_vq and the control vq.
+ */
+ if ((qid < first_vq || qid > last_vq) &&
+ !(enable_cvq && qid == vi->max_queue_pairs * 2))
+ return -EINVAL;
+
virtnet_fill_stats(vi, qid, ctx, p, false, hdr->type);
}
@@ -5073,7 +5081,8 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
if (enable_cvq)
virtnet_make_stat_req(vi, ctx, req, vi->max_queue_pairs * 2,
&j);
- ok = __virtnet_get_hw_stats(vi, ctx, req, sizeof(*req) * j, reply,
res_size);
+ ok = __virtnet_get_hw_stats(vi, ctx, req, sizeof(*req) * j, reply,
res_size,
+ first_vq, last_vq, enable_cvq);
kfree(req);
kfree(reply);
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
--
2.56.0
