From: "Carlos López" <clopez@suse.de>
To: qemu-devel@nongnu.org
Cc: "Carlos López" <clopez@suse.de>, "Michael S. Tsirkin" <mst@redhat.com>
Subject: [PATCH] virtio: fix reachable assertion due to stale value of cached region size
Date: Wed, 15 Feb 2023 23:14:46 +0100 [thread overview]
Message-ID: <20230215221444.29845-1-clopez@suse.de> (raw)
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.
Fixes: 9796d0ac8fb0 ("virtio: use address_space_map/unmap to access descriptors")
Signed-off-by: Carlos López <clopez@suse.de>
---
hw/virtio/virtio.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f35178f5fc..db70c4976e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1071,6 +1071,7 @@ static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
VirtIODevice *vdev = vq->vdev;
unsigned int max, idx;
unsigned int total_bufs, in_total, out_total;
+ MemoryRegionCache *desc_cache;
MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
int64_t len = 0;
int rc;
@@ -1078,15 +1079,13 @@ 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;
+ unsigned int num_bufs = total_bufs;
VRingDesc desc;
unsigned int i;
- num_bufs = total_bufs;
+ desc_cache = &caches->desc;
+ max = vq->vring.num;
if (!virtqueue_get_head(vq, idx++, &i)) {
goto err;
@@ -1218,14 +1217,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;
desc_cache = &caches->desc;
+ max = vq->vring.num;
+
vring_packed_desc_read(vdev, &desc, desc_cache, idx, true);
if (!is_desc_avail(desc.flags, wrap_counter)) {
break;
--
2.35.3
next reply other threads:[~2023-02-15 22:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-15 22:14 Carlos López [this message]
2023-03-01 22:03 ` [PATCH] virtio: fix reachable assertion due to stale value of cached region size Michael S. Tsirkin
2023-03-02 9:57 ` Carlos López
2023-03-02 6:58 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230215221444.29845-1-clopez@suse.de \
--to=clopez@suse.de \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).