111 lines
4.2 KiB
Diff
111 lines
4.2 KiB
Diff
From fc3c5fc2f3ccc236a6bcb670043912ab31e99772 Mon Sep 17 00:00:00 2001
|
|
From: wangmeiyang <wangmeiyang@xfusion.com>
|
|
Date: Fri, 26 May 2023 11:09:19 +0800
|
|
Subject: [PATCH] virtio: fix reachable assertion due to stale value of cached
|
|
region size
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
In virtqueue_{split,packed}_get_avail_bytes() descriptors are read
|
|
in a loop via MemoryRegionCache regions and calls to
|
|
vring_{split,packed}_desc_read() - these take a region cache and the
|
|
index of the descriptor to be read.
|
|
|
|
For direct descriptors we use a cache provided by the caller, whose
|
|
size matches that of the virtqueue vring. We limit the number of
|
|
descriptors we can read by the size of that vring:
|
|
max = vq->vring.num;
|
|
...
|
|
MemoryRegionCache *desc_cache = &caches->desc;
|
|
|
|
For indirect descriptors, we initialize a new cache and limit the
|
|
number of descriptors by the size of the intermediate descriptor:
|
|
|
|
len = address_space_cache_init(&indirect_desc_cache,
|
|
vdev->dma_as,
|
|
desc.addr, desc.len, false);
|
|
desc_cache = &indirect_desc_cache;
|
|
...
|
|
max = desc.len / sizeof(VRingDesc);
|
|
|
|
However, the first initialization of `max` is done outside the loop
|
|
where we process guest descriptors, while the second one is done
|
|
inside. This means that a sequence of an indirect descriptor followed
|
|
by a direct one will leave a stale value in `max`. If the second
|
|
descriptor's `next` field is smaller than the stale value, but
|
|
greater than the size of the virtqueue ring (and thus the cached
|
|
region), a failed assertion will be triggered in
|
|
address_space_read_cached() down the call chain.
|
|
|
|
Fix this by initializing `max` inside the loop in both functions.
|
|
|
|
origin commit: https://gitlab.com/qemu-project/qemu/-/commit/bbc1c327d7974261c61566cdb950cc5fa0196b41
|
|
Signed-off-by: Meiyang Wang <wangmeiyang@xfusion.com>
|
|
Fixes: 9796d0ac8fb0 ("virtio: use address_space_map/unmap to access descriptors")
|
|
Signed-off-by: Carlos López <clopez@suse.de>
|
|
Message-Id: <20230302100358.3613-1-clopez@suse.de>
|
|
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
|
|
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
|
|
---
|
|
hw/virtio/virtio.c | 11 +++++------
|
|
1 file changed, 5 insertions(+), 6 deletions(-)
|
|
|
|
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
|
|
index f8ab48e6bd..071668e3e0 100644
|
|
--- a/hw/virtio/virtio.c
|
|
+++ b/hw/virtio/virtio.c
|
|
@@ -983,7 +983,7 @@ static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
|
|
VRingMemoryRegionCaches *caches)
|
|
{
|
|
VirtIODevice *vdev = vq->vdev;
|
|
- unsigned int max, idx;
|
|
+ unsigned int idx;
|
|
unsigned int total_bufs, in_total, out_total;
|
|
MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
|
|
int64_t len = 0;
|
|
@@ -992,13 +992,12 @@ static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
|
|
idx = vq->last_avail_idx;
|
|
total_bufs = in_total = out_total = 0;
|
|
|
|
- max = vq->vring.num;
|
|
-
|
|
while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
|
|
MemoryRegionCache *desc_cache = &caches->desc;
|
|
unsigned int num_bufs;
|
|
VRingDesc desc;
|
|
unsigned int i;
|
|
+ unsigned int max = vq->vring.num;
|
|
|
|
num_bufs = total_bufs;
|
|
|
|
@@ -1120,7 +1119,7 @@ static void virtqueue_packed_get_avail_bytes(VirtQueue *vq,
|
|
VRingMemoryRegionCaches *caches)
|
|
{
|
|
VirtIODevice *vdev = vq->vdev;
|
|
- unsigned int max, idx;
|
|
+ unsigned int idx;
|
|
unsigned int total_bufs, in_total, out_total;
|
|
MemoryRegionCache *desc_cache;
|
|
MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
|
|
@@ -1132,14 +1131,14 @@ static void virtqueue_packed_get_avail_bytes(VirtQueue *vq,
|
|
wrap_counter = vq->last_avail_wrap_counter;
|
|
total_bufs = in_total = out_total = 0;
|
|
|
|
- max = vq->vring.num;
|
|
-
|
|
for (;;) {
|
|
unsigned int num_bufs = total_bufs;
|
|
unsigned int i = idx;
|
|
int rc;
|
|
+ unsigned int max = vq->vring.num;
|
|
|
|
desc_cache = &caches->desc;
|
|
+
|
|
vring_packed_desc_read(vdev, &desc, desc_cache, idx, true);
|
|
if (!is_desc_avail(desc.flags, wrap_counter)) {
|
|
break;
|
|
--
|
|
2.41.0.windows.1
|
|
|