pci, vhost, virtio, iommu: bugfixes
changes from v1:
- added a patch by Stefan
- attribution tag tweaks
The following changes since commit 300438ffbb8d9430cac2fcc15cba6f482b2c0587:
Merge tag 'pull-trivial-patches' of https://gitlab.com/mjt0k/qemu into staging (2026-07-24 09:13:49 -0400)
are available in the Git repository at:
https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
for you to fetch changes up to 6682ea3391277e732a6d74c5758206ba834e1615:
virtio: avoid packed vring virtio_queue_empty() infinite loops (CVE-2026-16457) (2026-07-27 15:14:16 -0400)
----------------------------------------------------------------
pci, vhost, virtio, iommu: bugfixes
Fixes all over the place, including a bunch of CVE fixes.
----------------------------------------------------------------
Clément MATHIEU--DRIF (1):
intel_iommu: Check address mask before using it in pasid-based iotlb invalidation
GuoHan Zhao (1):
hw/virtio/vdpa-dev: pass set_config buffer to vhost backend
Haotian Jiang (1):
hw/cxl: fix OOB access in cxl_doe_cdat_rsp via entry_handle
Laurent Vivier (4):
hw/virtio: reject zero-length packed indirect descriptor table hw/net/virtio-net: Protect from DMA re-entrancy bugs hw/virtio-rng: Fix host use-after-free (CVE-2026-50624) backends/rng: cap request size to avoid oversized allocation
Manos Pitsidianakis (2):
virtio-snd: check rx buffer descriptor size
virtio-snd: check for overflow before g_malloc0
Michael S. Tsirkin (17):
virtio: use masked features with set_features_ex
virtio-net: fix OOB read in RSC receive path
virtio-net: fix short frame OOB read in receive_filter()
libvhost-user: protect against OOB writes in vu_set_inflight_fd
libvhost-user: protect against OOB vring queue access
vhost: do not crash on ring map failure
virtio-scsi: fix SCSIRequest leak on a bad request
virtio-mmio: fix QUEUE_NUM_MAX
virtio: fix queue size validation against allocated maximum
virtio: stop migrating num_default, validate vring.num on load
virtio: fail early on bad config_len in migration
vhost-user: assert nregions within limit
virtio-pmem: wait for flush requests on unrealize
libvhost-user: validate last_batch_head in vu_check_queue_inflights
libvhost-user: fix heap overflow in vu_check_queue_inflights
libvduse: validate vq size
virtio-iommu: fix OOM due to unbounded call_rcu
Peter Maydell (3):
hw/pci-host/q35.c: Always initialize smram-region even if SMM disabled hw/pci-host/q35.c: Factor out creation of SMRAM MRs hw/pci-host/q35.c: Avoid early return in mch_write_config()
Stefan Hajnoczi (1):
virtio: avoid packed vring virtio_queue_empty() infinite loops (CVE-2026-16457)
include/hw/scsi/scsi.h | 1 + include/hw/virtio/virtio-bus.h | 1 + include/hw/virtio/virtio-iommu.h | 1 + include/hw/virtio/virtio-mmio.h | 1 + include/hw/virtio/virtio-pmem.h | 1 + include/system/rng.h | 14 ++++ backends/rng.c | 21 +++++- hw/audio/virtio-snd.c | 24 +++++-- hw/core/machine.c | 1 + hw/i386/intel_iommu.c | 9 +++ hw/mem/cxl_type3.c | 3 + hw/net/virtio-net.c | 25 ++++++- hw/pci-bridge/cxl_upstream.c | 3 + hw/pci-host/q35.c | 110 ++++++++++++++++-------------- hw/scsi/scsi-bus.c | 7 ++ hw/scsi/virtio-scsi.c | 2 + hw/virtio/vdpa-dev.c | 2 +- hw/virtio/vhost-user.c | 3 + hw/virtio/vhost.c | 5 +- hw/virtio/virtio-iommu.c | 29 ++++++++ hw/virtio/virtio-mmio.c | 7 +- hw/virtio/virtio-pmem.c | 17 ++++- hw/virtio/virtio-rng.c | 2 + hw/virtio/virtio.c | 47 +++++++++---- subprojects/libvduse/libvduse.c | 5 ++ subprojects/libvhost-user/libvhost-user.c | 61 ++++++++++++++++- 26 files changed, 317 insertions(+), 85 deletions(-)
[PULL v2 18/30] virtio-iommu: fix OOM due to unbounded call_rcu
Currently, within virtio-iommu, handle_command processes the command vq without any limits on the number of entries processed.
This can easily and repeatedly enable/disable multiple memory regions. Within the memory code, this causes an accumulation of an unbounded number of RCU-deferred FlatViews - each of these is supposed to be freed with call_rcu, but that never happens because the main thread never returns to the main loop.
Given FlatView is big, it's easy to have this balloon out to multiple Gigabytes of memory.
Limit the loop defer any remaining work to a timer.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
Message-ID: <eb46ab360dbe28c29cfa78812a7440dcb7444d59.1784807826.git.mst@redhat.com>
--- include/hw/virtio/virtio-iommu.h | 1 + hw/virtio/virtio-iommu.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)
include/hw/virtio/virtio-iommu.h +1 -0
@@ -65,6 +65,7 @@ struct VirtIOIOMMU {GTree *domains;QemuRecMutex mutex;GTree *endpoints; +QEMUTimer *cmd_timer;bool boot_bypass;Notifier machine_done;bool granule_frozen;
hw/virtio/virtio-iommu.c +29 -0
@@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,return ret ? ret :virtio_iommu_probe (s, &req, buf); } +static void virtio_iommu_handle_command (VirtIODevice *vdev,VirtQueue *vq); + +static void virtio_iommu_handle_command_timer (void *opaque) +{ +VirtIOIOMMU *s = opaque; +VirtIODevice *vdev =VIRTIO_DEVICE (s); + +if (virtio_device_started (vdev, vdev->status) && !vdev->broken) { +virtio_iommu_handle_command (vdev, s->req_vq); + } +} +static void virtio_iommu_handle_command (VirtIODevice *vdev,VirtQueue *vq) {VirtIOIOMMU *s =VIRTIO_IOMMU (vdev);
@@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)struct iovec *iov;void *buf =NULL ;size_t sz; +unsigned int batch =0 ;for (;;) {size_t output_size =sizeof (tail); +if (++batch >virtio_queue_get_num (vdev,virtio_get_queue_index (vq))) { +timer_mod (s->cmd_timer, +qemu_clock_get_ns (QEMU_CLOCK_VIRTUAL_RT ) +1 ); +break ; + } + elem =virtqueue_pop (vq,sizeof (VirtQueueElement));if (!elem) {return ;
@@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp) s->req_vq =virtio_add_queue (vdev,VIOMMU_DEFAULT_QUEUE_SIZE , virtio_iommu_handle_command); s->event_vq =virtio_add_queue (vdev,VIOMMU_DEFAULT_QUEUE_SIZE ,NULL ); + s->cmd_timer =timer_new_ns (QEMU_CLOCK_VIRTUAL_RT , + virtio_iommu_handle_command_timer, s);/* * config.bypass is needed to get initial address space early, such as
@@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)qemu_rec_mutex_destroy (&s->mutex); +timer_free (s->cmd_timer);virtio_delete_queue (s->req_vq);virtio_delete_queue (s->event_vq);virtio_cleanup (vdev);
@@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)trace_virtio_iommu_device_reset_exit (); +timer_del (s->cmd_timer); +if (s->domains) {g_tree_destroy (s->domains); }
@@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)* still correct. */ virtio_iommu_switch_address_space_all (s); + +if (virtio_device_started (VIRTIO_DEVICE (s),VIRTIO_DEVICE (s)->status)) { +timer_mod (s->cmd_timer, +qemu_clock_get_ns (QEMU_CLOCK_VIRTUAL_RT ) +1 ); + }return 0 ; }
Signature · 1 line
MST
[PULL v2 25/30] hw/cxl: fix OOB access in cxl_doe_cdat_rsp via entry_handle
In reply to Michael S. TsirkinFrom: Haotian Jiang <[email protected]>
cxl_doe_cdat_rsp() takes ent = req->entry_handle (uint16_t, fully guest-controlled, 0..0xFFFF) and directly indexes cdat->entry[ent] without checking ent < cdat->entry_len. For a default cxl-type3 with one volatile memory region, entry_len = 1 + CT3_CDAT_NUM_ENTRIES = 7, so any entry_handle >= 7 reads past the CDATEntry array into host heap.
The OOB-read base/length are then used inmemcpy(read_mbox + offset, base, len) at cxl_type3.c:298-299, leaking host heap memory to the guest via PCI_EXP_DOE_RD_DATA_MBOX, and potentially overflowing the 1 MiB read_mbox heap buffer when the OOB length field is large.
The same bug exists in the cxl-upstream implementation.
Existing checks do not bound ent: assert(cdat->entry_len) only ensures the table is loaded; the minimum-length check only guards against a truncated CDATReq; the entry_handle ternary at line 293 only decides the next-handle echo, not the current access; pcie_doe_get_obj_len reads header.length, not entry_handle.
Reproduce: build QEMU with CONFIG_CXL, boot
-M q35,cxl=on -device pxb-cxl,bus_nr=52 ... -device cxl-type3,...
then send a CDATReq with entry_handle=0xFFFF via the DOE mailbox at config offset 0x190. Under ASAN this reports SEGV in cxl_doe_cdat_rsp at cxl_type3.c:281.
Message-ID: <[email protected]>
--- hw/mem/cxl_type3.c | 3 +++ hw/pci-bridge/cxl_upstream.c | 3 +++ 2 files changed, 6 insertions(+)
hw/mem/cxl_type3.c +3 -0
@@ -278,6 +278,9 @@ static bool cxl_doe_cdat_rsp(DOECap *doe_cap) } ent = req->entry_handle; +if (ent >= cdat->entry_len) { +return false; + } base = cdat->entry[ent].base; len = cdat->entry[ent].length;
hw/pci-bridge/cxl_upstream.c +3 -0
@@ -158,6 +158,9 @@ static bool cxl_doe_cdat_rsp(DOECap *doe_cap) } ent = req->entry_handle; +if (ent >= cdat->entry_len) { +return false; + } base = cdat->entry[ent].base; len = cdat->entry[ent].length;
Signature · 1 line
MST
[PULL v2 02/30] virtio-net: fix OOB read in RSC receive path
In reply to Michael S. TsirkinThe RSC receive path parses incoming frames at guest_hdr_len byte offsets, but the backend buffer contains only host_hdr_len bytes of vnet header. If the lengths differ, RSC would read at the wrong offset and cause an OOB read.
This is no longer possible after the previous patch, but the assumption seem fragile. Along the defense in depth lines, let's validate. To ensure we are not breaking any valid setups by mistake, warn and fall back to the normal receive path when host_hdr_len != guest_hdr_len.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3623
Message-ID: <f261dcd535edc890f8636d8ae5ac1007bc32b8b4.1784891251.git.mst@redhat.com>
--- hw/net/virtio-net.c | 7 +++++++ 1 file changed, 7 insertions(+)
hw/net/virtio-net.c +7 -0
@@ -2674,6 +2674,13 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, {VirtIONet *n =qemu_get_nic_opaque (nc);if ((n->rsc4_enabled || n->rsc6_enabled)) { +/* this never happens with existing backends, but just in case. */ +if (n->host_hdr_len != n->guest_hdr_len) { +warn_report_once ("virtio-net: host_hdr_len %zu != guest_hdr_len %zu, " +"skipping RSC" , + n->host_hdr_len, n->guest_hdr_len); +return virtio_net_do_receive (nc, buf, size); + }return virtio_net_rsc_receive (nc, buf, size); }else {return virtio_net_do_receive (nc, buf, size);
Signature · 1 line
MST
[PULL v2 05/30] libvhost-user: protect against OOB vring queue access
In reply to Michael S. TsirkinSET_VRING_NUM, SET_VRING_ADDR, SET_VRING_BASE, and GET_VRING_BASE handlers all use the queue index from the message to access dev->vq[] without checking that it is below dev->max_queues, so a malformed message causes an out-of-bounds heap access.
Frontend is trusted so not a security problem, but an OOB access is not a nice way to handle errors. Check, and panic.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3741
Message-ID: <dbba777b25f86587c8d131891a2b4b2be97e5c5b.1784899069.git.mst@redhat.com>
--- subprojects/libvhost-user/libvhost-user.c | 28 ++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
subprojects/libvhost-user/libvhost-user.c +27 -1
@@ -1200,6 +1200,12 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)DPRINT ("State.index: %u\n" , index);DPRINT ("State.num: %u\n" , num); + +if (index >= dev->max_queues) { +vu_panic (dev,"Invalid vring_num index: %u" , index); +return false; + } + dev->vq[index].vring.num = num;return false;
@@ -1210,7 +1216,7 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg) {struct vhost_vring_addr addr = vmsg->payload.addr, *vra = &addr;unsigned int index = vra->index; -VuVirtq *vq = &dev->vq[index]; +VuVirtq *vq;DPRINT ("vhost_vring_addr:\n" );DPRINT (" index: %d\n" , vra->index);
@@ -1220,6 +1226,12 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)DPRINT (" avail_user_addr: 0x%016" PRIx64"\n" , (uint64_t )vra->avail_user_addr);DPRINT (" log_guest_addr: 0x%016" PRIx64"\n" , (uint64_t )vra->log_guest_addr); +if (index >= dev->max_queues) { +vu_panic (dev,"Invalid vring_addr index: %u" , index); +return false; + } + + vq = &dev->vq[index]; vq->vra = *vra; vq->vring.flags = vra->flags; vq->vring.log_guest_addr = vra->log_guest_addr;
@@ -1256,6 +1268,12 @@ vu_set_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)DPRINT ("State.index: %u\n" , index);DPRINT ("State.num: %u\n" , num); + +if (index >= dev->max_queues) { +vu_panic (dev,"Invalid vring_base index: %u" , index); +return false; + } + dev->vq[index].shadow_avail_idx = dev->vq[index].last_avail_idx = num;return false;
@@ -1267,6 +1285,14 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)unsigned int index = vmsg->payload.state.index;DPRINT ("State.index: %u\n" , index); + +if (index >= dev->max_queues) { +vu_panic (dev,"Invalid vring_base index: %u" , index); + vmsg->payload.state.num =0 ; + vmsg->size =sizeof (vmsg->payload.state); +return true; + } + vmsg->payload.state.num = dev->vq[index].last_avail_idx; vmsg->size =sizeof (vmsg->payload.state);
Signature · 1 line
MST
[PULL v2 06/30] hw/virtio: reject zero-length packed indirect descriptor table
In reply to Michael S. TsirkinFrom: Laurent Vivier <[email protected]>
The split-ring path already rejects a zero-length indirect descriptor table since commit 7423192912af ("virtio: add checks for the size of the indirect table"). The packed-ring path is missing the same check, allowing a guest to trigger an assertion in address_space_cache_init() with a packed indirect descriptor that has len=0.
Add the same !desc.len check to the packed-ring indirect validation in both virtqueue_packed_get_avail_bytes() and virtqueue_packed_pop().
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3984
Message-ID: <[email protected]>
--- hw/virtio/virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
hw/virtio/virtio.c +2 -2
@@ -1475,7 +1475,7 @@ static void virtqueue_packed_get_avail_bytes(VirtQueue *vq, }if (desc.flags &VRING_DESC_F_INDIRECT ) { -if (desc.len %sizeof (VRingPackedDesc)) { +if (!desc.len || (desc.len %sizeof (VRingPackedDesc))) {virtio_error (vdev,"Invalid size for indirect buffer table" ); goto err; }
@@ -1927,7 +1927,7 @@ static void *virtqueue_packed_pop(VirtQueue *vq, size_t sz)vring_packed_desc_read (vdev, &desc, desc_cache, i, true); id = desc.id;if (desc.flags &VRING_DESC_F_INDIRECT ) { -if (desc.len %sizeof (VRingPackedDesc)) { +if (!desc.len || (desc.len %sizeof (VRingPackedDesc))) {virtio_error (vdev,"Invalid size for indirect buffer table" ); goto done; }
Signature · 1 line
MST
[PULL v2 21/30] hw/pci-host/q35.c: Always initialize smram-region even if SMM disabled
In reply to Michael S. TsirkinFrom: Peter Maydell <[email protected]>
The MCHPCIState::smram_region looks like it ought to be SMM-specific, but it isn't, because its behaviour is "alias the PCI address space into system memory at the SMRAM_C_BASE offset", and it must be enabled for "hide SMRAM", and disabled for "show SMRAM". If the SMRAM regions are disabled, we want "hide SMRAM", so we need to initialize and place this MR. Do this in the minimal way, by moving the "bail out of realize if has_smm_ranges is false" check down below the initialization code.
This fixes a bug where disabling SMM causes the VGA screen to be blank during seabios output, until the OS graphics driver is initialized. This is most obvious for accelerators which have no SMM support (e.g. NVMM, HVF, WHPX) as there smm=off is the default, but you can also see it on KVM and TCG if you explicitly pass smm=off:
qemu-system-x86_64 -machine q35,accel=kvm,smm=off
The early return is bug-prone, so we can refactor the code to clean it up, but this is the minimal bug fix for backports, and is what Debian used to work around this:
https://salsa.debian.org/qemu-team/qemu/-/commit/6e0766f0f897dc2b75ab87dd59da0d4639bb37ee
Another proposed fix for this:
https://patchew.org/QEMU/[email protected]/
also moves an early return in mch_update-smram() and tweaks mch_update_smram() accordingly. This shouldn't be necessary, because in the no-SMM case smram_region should always be enabled and we don't want to allow the guest to make it disabled.
NetBSD bug: https://gnats.NetBSD.org/59721
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2608
Message-ID: <[email protected]>
--- hw/pci-host/q35.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
hw/pci-host/q35.c +12 -5
@@ -596,11 +596,14 @@ static void mch_realize(PCIDevice *d, Error **errp) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); } -if (!mch->has_smm_ranges) { -return ; - } - -/* if *disabled* show SMRAM to all CPUs */ +/* +* This memory region looks like it's SMM specific, but it is not. +* It's an alias that makes the pci_address_space appear in system +* memory at the SMRAM_C_BASE address. The alias is enabled when the +* CPU should not see SMRAM, and *disabled* when the low SMRAM should be +* visible. So for non-SMM configs we need to create the alias, and +* leave it permanently enabled. +*/ memory_region_init_alias (&mch->smram_region,OBJECT (mch),"smram-region" , mch->pci_address_space,MCH_HOST_BRIDGE_SMRAM_C_BASE ,MCH_HOST_BRIDGE_SMRAM_C_SIZE );
@@ -608,6 +611,10 @@ static void mch_realize(PCIDevice *d, Error **errp) &mch->smram_region, 1);memory_region_set_enabled (&mch->smram_region, true); +if (!mch->has_smm_ranges) { +return ; + } +memory_region_init_alias (&mch->open_high_smram,OBJECT (mch),"smram-open-high" , mch->ram_memory,MCH_HOST_BRIDGE_SMRAM_C_BASE ,MCH_HOST_BRIDGE_SMRAM_C_SIZE );
Signature · 1 line
MST
[PULL v2 17/30] libvduse: validate vq size
In reply to Michael S. Tsirkinlibvduse assumes that vq size (aka vq num) is below VIRTQUEUE_MAX_SIZE and maps logs large enough based on this assumption.
However, vduse_queue_enable() accepts the vq size returned through VDUSE_VQ_GET_INFO without validation, so a value above VIRTQUEUE_MAX_SIZE (1024) overruns the inflight log and causes out-of-bounds writes in vduse_queue_inflight_get().
According to the virtio spec, vq size can only be reduced, not increased, so vq size must not exceed the previously configured max_size, but the kernel vduse module does not validate this for us, and we should not trust another process to follow the spec.
Validate and reject vq size values above VIRTQUEUE_MAX_SIZE.
Fixes: CVE-2026-61402
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3652
Message-ID: <bf7e71b3139875e5e00fd53970c772d6c90dc2a1.1784888961.git.mst@redhat.com>
--- subprojects/libvduse/libvduse.c | 5 +++++ 1 file changed, 5 insertions(+)
subprojects/libvduse/libvduse.c +5 -0
@@ -902,6 +902,11 @@ static void vduse_queue_enable(VduseVirtq *vq)return ; } +if (vq_info.num >VIRTQUEUE_MAX_SIZE ) { +fprintf (stderr,"vq[%d] vring num %u exceeds max %u\n" , + vq->index, vq_info.num,VIRTQUEUE_MAX_SIZE ); +return ; + } vq->vring.num = vq_info.num; vq->vring.desc_addr = vq_info.desc_addr; vq->vring.avail_addr = vq_info.driver_addr;
Signature · 1 line
MST
[PULL v2 04/30] libvhost-user: protect against OOB writes in vu_set_inflight_fd
In reply to Michael S. Tsirkinvu_set_inflight_fd() trusts the num_queues value from the VHOST_USER_SET_INFLIGHT_FD message without checking it against dev->max_queues, so an oversized value causes out-of-bounds writes to dev->vq.
Front end is generally trusted so not a security problem, but OOB isn't a nice way to handle frontend bugs. Let's harden this a bit:
check num_queues and panic if it's invalid.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3740
Message-ID: <23b3f12388c1035f208550df9de9944c22d8d534.1784899127.git.mst@redhat.com>
--- subprojects/libvhost-user/libvhost-user.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
subprojects/libvhost-user/libvhost-user.c +20 -0
@@ -1998,6 +1998,8 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)if (vmsg->size !=sizeof (vmsg->payload.inflight)) {vu_panic (dev,"Invalid get_inflight_fd message:%d" , vmsg->size); +vmsg_close_fds (vmsg); + vmsg->fd_num =0 ; vmsg->payload.inflight.mmap_size =0 ;return true; }
@@ -2005,6 +2007,15 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg) num_queues = vmsg->payload.inflight.num_queues; queue_size = vmsg->payload.inflight.queue_size; +if (num_queues > dev->max_queues) { +vu_panic (dev,"Invalid get_inflight_fd num_queues: %" PRId16, + num_queues); +vmsg_close_fds (vmsg); + vmsg->fd_num =0 ; + vmsg->payload.inflight.mmap_size =0 ; +return true; + } +DPRINT ("set_inflight_fd num_queues: %" PRId16"\n" , num_queues);DPRINT ("set_inflight_fd queue_size: %" PRId16"\n" , queue_size);
@@ -2052,6 +2063,7 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg) vmsg->size != sizeof(vmsg->payload.inflight)) {vu_panic (dev,"Invalid set_inflight_fd message size:%d fds:%d" , vmsg->size, vmsg->fd_num); +vmsg_close_fds (vmsg);return false; }
@@ -2061,6 +2073,13 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg) num_queues = vmsg->payload.inflight.num_queues; queue_size = vmsg->payload.inflight.queue_size; +if (num_queues > dev->max_queues) { +vu_panic (dev,"Invalid set_inflight_fd num_queues: %" PRId16, + num_queues); +close (fd); +return false; + } +DPRINT ("set_inflight_fd mmap_size: %" PRId64"\n" , mmap_size);DPRINT ("set_inflight_fd mmap_offset: %" PRId64"\n" , mmap_offset);DPRINT ("set_inflight_fd num_queues: %" PRId16"\n" , num_queues);
@@ -2071,6 +2090,7 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)if (rc ==MAP_FAILED ) {vu_panic (dev,"set_inflight_fd mmap error: %s" ,strerror (errno)); +close (fd);return false; }
Signature · 1 line
MST
[PULL v2 01/30] virtio: use masked features with set_features_ex
In reply to Michael S. Tsirkinvirtio_set_features_nocheck() calls set_features_ex with guest-supplied feature bits, without masking the value with host features (unlike set_features which gets the correct val & host_features).
This does not matter if the driver matches spec, but drivers can be malicious or buggy and set bit outside the host mask.
Devices don't expect this, so unsupported guest feature bits getting set can break the host. In virtio-net, this can enable RSC without vnet header support and cause out-of-bounds reads from short packets.
Pass the masked features to set_features_ex, consistent with set_features.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3623
Message-ID: <dfd27c9b26e442a2076f6ddc9bb3d38363d9b2da.1784891251.git.mst@redhat.com>
--- hw/virtio/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
hw/virtio/virtio.c +1 -1
@@ -3320,7 +3320,7 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)virtio_features_and (tmp, val, vdev->host_features_ex);if (k->set_features_ex) { - k->set_features_ex (vdev, val); + k->set_features_ex (vdev, tmp); }else if (k->set_features) { bad = bad ||virtio_features_use_ex (tmp); k->set_features (vdev, tmp[0 ]);
Signature · 1 line
MST
Paolo Abeni
Acked-byRe: [PULL v2 01/30] virtio: use masked features with set_features_ex
Quoted 32 lines · Michael S. Tsirkin
virtio_set_features_nocheck()callsset_features_exwith guest-supplied feature bits, without masking the value with host features (unlikeset_featureswhich gets the correct val &host_features).This does not matter if the driver matches spec, but drivers can be malicious or buggy and set bit outside the host mask.
Devices don't expect this, so unsupported guest feature bits getting set can break the host. In virtio-net, this can enable RSC without vnet header support and cause out-of-bounds reads from short packets.
Pass the masked features to
set_features_ex, consistent withset_features.Fixes CVE-2026-63321Fixes 64a6a336f4 ("virtio: add support for negotiating extended features")Cc Jason Wang <[email protected]>Cc Yuri Benditovich <[email protected]>Cc Paolo Abeni <[email protected]>Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3623
Reported-by huntr bubble <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>Message-ID: <dfd27c9b26e442a2076f6ddc9bb3d38363d9b2da.1784891251.git.mst@redhat.com>
--- hw/virtio/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)hw/virtio/virtio.c +0
-0
hw/virtio/virtio.c static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val) · line 3326
Quoted 8 lines
hw/virtio/virtio.c +1
-1@@ -3320,7 +3320,7 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)virtio_features_and (tmp, val, vdev->host_features_ex);if (k->set_features_ex) { - k->set_features_ex (vdev, val); + k->set_features_ex (vdev, tmp); }else if (k->set_features) { bad = bad ||virtio_features_use_ex (tmp); k->set_features (vdev, tmp[0 ]);
I fear this comes too late, but FWIW the fix LGTM, thanks Michael!
[PULL v2 22/30] hw/pci-host/q35.c: Factor out creation of SMRAM MRs
In reply to Michael S. TsirkinFrom: Peter Maydell <[email protected]>
mch_realize has a large section that deals with initializing the SMRAM-specific MemoryRegions. Currently we do an early return from the realize function if mch->has_smm_ranges is false, but this has the potential for bugs if somebody adds new code at the end of the function that isn't SMM-specific. Pull the MR init code out into its own function, so we can do the smm-ranges specific handling in the realize function in a more obvious way.
This commit shouldn't change behaviour at all.
Message-ID: <[email protected]>
--- hw/pci-host/q35.c | 91 ++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 44 deletions(-)
hw/pci-host/q35.c +47 -44
@@ -572,49 +572,9 @@ static void mch_reset(DeviceState *qdev)mch_update (mch); } -static void mch_realize (PCIDevice *d,Error **errp) +static void mch_init_smram_regions (MCHPCIState *mch) { -int i; -MCHPCIState *mch =MCH_PCI_DEVICE (d); - -if (mch->ext_tseg_mbytes >MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX ) { -error_setg (errp,"invalid extended-tseg-mbytes value: %" PRIu16, - mch->ext_tseg_mbytes); -return ; - } - -/* setup pci memory mapping */ -pc_pci_as_mapping_init (mch->system_memory, mch->pci_address_space); - -/* PAM */ -init_pam (&mch->pam_regions[0 ],OBJECT (mch), mch->ram_memory, - mch->system_memory, mch->pci_address_space, -PAM_BIOS_BASE ,PAM_BIOS_SIZE ); -for (i =0 ; i <ARRAY_SIZE (mch->pam_regions) -1 ; ++i) { -init_pam (&mch->pam_regions[i +1 ],OBJECT (mch), mch->ram_memory, - mch->system_memory, mch->pci_address_space, -PAM_EXPAN_BASE + i *PAM_EXPAN_SIZE ,PAM_EXPAN_SIZE ); - } - -/* -* This memory region looks like it's SMM specific, but it is not. -* It's an alias that makes the pci_address_space appear in system -* memory at the SMRAM_C_BASE address. The alias is enabled when the -* CPU should not see SMRAM, and *disabled* when the low SMRAM should be -* visible. So for non-SMM configs we need to create the alias, and -* leave it permanently enabled. -*/ -memory_region_init_alias (&mch->smram_region,OBJECT (mch),"smram-region" , - mch->pci_address_space,MCH_HOST_BRIDGE_SMRAM_C_BASE , -MCH_HOST_BRIDGE_SMRAM_C_SIZE ); -memory_region_add_subregion_overlap (mch->system_memory,MCH_HOST_BRIDGE_SMRAM_C_BASE , - &mch->smram_region,1 ); -memory_region_set_enabled (&mch->smram_region, true); - -if (!mch->has_smm_ranges) { -return ; - } - +/* Initialize all the SMRAM specific MemoryRegions */ memory_region_init_alias (&mch->open_high_smram,OBJECT (mch),"smram-open-high" , mch->ram_memory,MCH_HOST_BRIDGE_SMRAM_C_BASE ,MCH_HOST_BRIDGE_SMRAM_C_SIZE );
@@ -670,9 +630,52 @@ static void mch_realize(PCIDevice *d, Error **errp)memory_region_set_enabled (&mch->smbase_window, false);memory_region_add_subregion (&mch->smram,MCH_HOST_BRIDGE_SMBASE_ADDR , &mch->smbase_window); +} -object_property_add_const_link (qdev_get_machine (),"smram" , -OBJECT (&mch->smram)); +static void mch_realize (PCIDevice *d,Error **errp) +{ +int i; +MCHPCIState *mch =MCH_PCI_DEVICE (d); + +if (mch->ext_tseg_mbytes >MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_MAX ) { +error_setg (errp,"invalid extended-tseg-mbytes value: %" PRIu16, + mch->ext_tseg_mbytes); +return ; + } + +/* setup pci memory mapping */ +pc_pci_as_mapping_init (mch->system_memory, mch->pci_address_space); + +/* PAM */ +init_pam (&mch->pam_regions[0 ],OBJECT (mch), mch->ram_memory, + mch->system_memory, mch->pci_address_space, +PAM_BIOS_BASE ,PAM_BIOS_SIZE ); +for (i =0 ; i <ARRAY_SIZE (mch->pam_regions) -1 ; ++i) { +init_pam (&mch->pam_regions[i +1 ],OBJECT (mch), mch->ram_memory, + mch->system_memory, mch->pci_address_space, +PAM_EXPAN_BASE + i *PAM_EXPAN_SIZE ,PAM_EXPAN_SIZE ); + } + +/* +* This memory region looks like it's SMM specific, but it is not. +* It's an alias that makes the pci_address_space appear in system +* memory at the SMRAM_C_BASE address. The alias is enabled when the +* CPU should not see SMRAM, and *disabled* when the low SMRAM should be +* visible. So for non-SMM configs we need to create the alias, and +* leave it permanently enabled. +*/ +memory_region_init_alias (&mch->smram_region,OBJECT (mch),"smram-region" , + mch->pci_address_space,MCH_HOST_BRIDGE_SMRAM_C_BASE , +MCH_HOST_BRIDGE_SMRAM_C_SIZE ); +memory_region_add_subregion_overlap (mch->system_memory,MCH_HOST_BRIDGE_SMRAM_C_BASE , + &mch->smram_region,1 ); +memory_region_set_enabled (&mch->smram_region, true); + +if (mch->has_smm_ranges) { +mch_init_smram_regions (mch); +object_property_add_const_link (qdev_get_machine (),"smram" , +OBJECT (&mch->smram)); + } }static const Property mch_props[] = {
Signature · 1 line
MST
[PULL v2 10/30] virtio: fix queue size validation against allocated maximum
In reply to Michael S. Tsirkinvirtio_add_queue() allocates used_elems for num_default entries, but virtio_queue_set_num() accepts larger guest-supplied queue sizes up to VIRTQUEUE_MAX_SIZE. With VIRTIO_F_IN_ORDER, this lets the guest drive used_elems accesses past the allocation and cause out-of-bounds reads and writes.
Reject queue sizes larger than num_default in virtio_queue_set_num() and mark the device broken.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3882
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3921
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3923
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3613
Message-ID: <eb7cc3672a20db392f577edbece2300aa6754dd3.1784898967.git.mst@redhat.com>
--- hw/virtio/virtio.c | 5 +++++ 1 file changed, 5 insertions(+)
hw/virtio/virtio.c +5 -0
@@ -2418,6 +2418,11 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) num < 0) {return ; } +if (num > vdev->vq[n].vring.num_default) { +virtio_error (vdev,"virtio: queue %d size %d exceeds max size %u" , + n, num, vdev->vq[n].vring.num_default); +return ; + } vdev->vq[n].vring.num = num; }
Signature · 1 line
MST
[PULL v2 26/30] intel_iommu: Check address mask before using it in pasid-based iotlb invalidation
In reply to Michael S. TsirkinFrom: Clément MATHIEU--DRIF <[email protected]>
Prevent a buggy driver to execute malformed invalidation operations.
Add the same assert as in vtd_iotlb_page_invalidate.
Message-ID: <[email protected]>
--- hw/i386/intel_iommu.c | 9 +++++++++ 1 file changed, 9 insertions(+)
hw/i386/intel_iommu.c +9 -0
@@ -3021,6 +3021,8 @@ static void vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id, {VTDIOTLBPageInvInfo info; +assert (am <=VTD_MAMV ); + info.domain_id = domain_id; info.pasid = pasid; info.addr = addr;
@@ -3060,6 +3062,13 @@ static bool vtd_process_piotlb_desc(IntelIOMMUState *s,case VTD_INV_DESC_PIOTLB_PSI_IN_PASID : am =VTD_INV_DESC_PIOTLB_AM (inv_desc->val[1 ]); +if (am >VTD_MAMV ) { +error_report_once ("%s: invalid piotlb inv desc: hi=0x%" PRIx64 +", lo=0x%" PRIx64" (am=%u > VTD_MAMV=%llu)" , + __func__, inv_desc->val[1 ], inv_desc->val[0 ], + am,VTD_MAMV ); +return false; + } addr = (hwaddr )VTD_INV_DESC_PIOTLB_ADDR (inv_desc->val[1 ]);vtd_piotlb_page_invalidate (s, domain_id, pasid, addr, am,VTD_INV_DESC_PIOTLB_IH (inv_desc));
Signature · 1 line
MST
[PULL v2 24/30] hw/virtio/vdpa-dev: pass set_config buffer to vhost backend
In reply to Michael S. TsirkinFrom: GuoHan Zhao <[email protected]>
vhost_vdpa_device_set_config() receives the updated config buffer, but forwards s->config to the vhost backend. Since s->config is refreshed by get_config(), it may contain stale backend state.
Pass the supplied config buffer to vhost_dev_set_config() instead.
Message-ID: <[email protected]>
--- hw/virtio/vdpa-dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
hw/virtio/vdpa-dev.c +1 -1
@@ -212,7 +212,7 @@ vhost_vdpa_device_set_config(VirtIODevice *vdev, const uint8_t *config)VhostVdpaDevice *s =VHOST_VDPA_DEVICE (vdev);int ret; - ret =vhost_dev_set_config (&s->dev, s->config,0 , s->config_size, + ret =vhost_dev_set_config (&s->dev, config,0 , s->config_size,VHOST_SET_CONFIG_TYPE_FRONTEND );if (ret) {error_report ("set device config space failed" );
Signature · 1 line
MST
[PULL v2 12/30] virtio: fail early on bad config_len in migration
In reply to Michael S. Tsirkinvirtio_load() attempts to load config_len bytes from the migration stream. If that's huge (e.g. 4g) this will uselessly spin beyond the end of the stream for seconds. Not nice. Check qemu_file_get_error() and bail out early, instead.
Also note that config_len is int32_t but is coerced to unsigned when used. Switch it to uint32_t to make this clearer.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3891
Message-ID: <cfbefa358af5885eb386637216552bfeba5e7bbc.1784898922.git.mst@redhat.com>
--- hw/virtio/virtio.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
hw/virtio/virtio.c +4 -1
@@ -3505,7 +3505,7 @@ int coroutine_mixed_fn virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) {int i, ret; -int32_t config_len; +uint32_t config_len;uint32_t num;uint32_t features;BusState *qbus =qdev_get_parent_bus (DEVICE (vdev));
@@ -3553,6 +3553,9 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)qemu_get_buffer (f, vdev->config,MIN (config_len, vdev->config_len));while (config_len > vdev->config_len) { +if (qemu_file_get_error (f)) { +return -1 ; + }qemu_get_byte (f); config_len--; }
Signature · 1 line
MST
[PULL v2 03/30] virtio-net: fix short frame OOB read in receive_filter()
In reply to Michael S. TsirkinWithin virtio-net, receive_filter() reads Ethernet header fields without any length checks.
But virtio-net sets do_not_pad in NetClientState, so backends such as socket forward frames at the size supplied by the peer without padding to the Ethernet minimum. A short frame thus causes an out-of-bounds read.
Add size checks in receive_filter() and drop the truncated frames.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3626
Message-ID: <ee5c77b96ab66b2dd518f146def8727216a5c495.1784895727.git.mst@redhat.com>
--- hw/net/virtio-net.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
hw/net/virtio-net.c +12 -1
@@ -1744,10 +1744,21 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)if (n->promisc)return 1 ; +if (size < n->host_hdr_len +14 ) { +/* Truncated ethernet packet */ +return 0 ; + } + ptr += n->host_hdr_len;if (!memcmp (&ptr[12 ], vlan,sizeof (vlan))) { -int vid =lduw_be_p (ptr +14 ) &0xfff ; +int vid; + +/* Truncated vlan packet */ +if (size < n->host_hdr_len +16 ) { +return 0 ; + } + vid =lduw_be_p (ptr +14 ) &0xfff ;if (!(n->vlans[vid >>5 ] & (1U << (vid &0x1f ))))return 0 ; }
Signature · 1 line
MST
[PULL v2 11/30] virtio: stop migrating num_default, validate vring.num on load
In reply to Michael S. Tsirkinnum_default tracks the allocation size of used_elems, set by virtio_add_queue(). Migrating it via the ringsize subsection is
wrong: a migration stream (malicious or simply from a different configuration) can inflate num_default so that virtio_queue_set_num() accepts oversized values, leading to OOB access on the used_elems array.
It is not even migrated consistently: a configuration with a smaller num_default could thinkably migrate and work but in the common case of num == num_default the value is not actually sent.
Stop migrating num_default: make virtio_ringsize_needed() return false so the subsection is never sent, and use VMSTATE_UNUSED to consume the field from old streams without applying it. The destination keeps its local num_default from virtio_add_queue(), which matches the actual allocation.
Also validate vring.num against num_default when loading the core virtio state, rejecting streams that supply a queue size larger than the locally allocated maximum.
Message-ID: <3e6a7c403f93acc37af6a6332fdc65049ae218fb.1784894327.git.mst@redhat.com>
--- hw/virtio/virtio.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
hw/virtio/virtio.c +7 -9
@@ -2805,14 +2805,6 @@ static bool virtio_packed_virtqueue_needed(void *opaque)static bool virtio_ringsize_needed (void *opaque) { -VirtIODevice *vdev = opaque; -int i; - -for (i =0 ; i <VIRTIO_QUEUE_MAX ; i++) { -if (vdev->vq[i].vring.num != vdev->vq[i].vring.num_default) { -return true; - } - }return false; }
@@ -2901,7 +2893,7 @@ static const VMStateDescription vmstate_ringsize = { .version_id = 1, .minimum_version_id = 1, .fields = (const VMStateField []) { - VMSTATE_UINT32(vring.num_default, struct VirtQueue), +VMSTATE_UNUSED (sizeof (uint32_t )),VMSTATE_END_OF_LIST () } };
@@ -3581,6 +3573,12 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)for (i =0 ; i < num; i++) { vdev->vq[i].vring.num =qemu_get_be32 (f); +if (vdev->vq[i].vring.num > vdev->vq[i].vring.num_default) { +error_report ("VQ %d vring.num %u exceeds allocated max %u" , + i, vdev->vq[i].vring.num, + vdev->vq[i].vring.num_default); +return -1 ; + }if (k->has_variable_vring_alignment) { vdev->vq[i].vring.align =qemu_get_be32 (f); }
Signature · 1 line
MST
[PULL v2 14/30] virtio-pmem: wait for flush requests on unrealize
In reply to Michael S. Tsirkinvirtio_pmem_flush submits fsync requests to the thread pool and stores a VirtIOPMEM pointer in each request. If device is deleted e.g. by hot-unplug, once these complete, done_cb can run after virtio_pmem_unrealize frees the device, causing a use-after-free.
Track in-flight requests and wait in virtio_pmem_unrealize until their completions finish before tearing the device down.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3938
Message-ID: <417b6685f37ce818c660ca3c84945992f5c30dcf.1784894206.git.mst@redhat.com>
--- include/hw/virtio/virtio-pmem.h | 1 + hw/virtio/virtio-pmem.c | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-)
include/hw/virtio/virtio-pmem.h +1 -0
@@ -32,6 +32,7 @@ struct VirtIOPMEM {VirtQueue *rq_vq;uint64_t start;HostMemoryBackend *memdev; +unsigned int inflight; };struct VirtIOPMEMClass {
hw/virtio/virtio-pmem.c +15 -2
@@ -23,6 +23,7 @@#include "standard-headers/linux/virtio_pmem.h" #include "system/hostmem.h" #include "block/thread-pool.h" +#include "qemu/aio-wait.h" #include "trace.h" typedef struct VirtIODeviceRequest {
@@ -54,14 +55,20 @@ static int worker_cb(void *opaque)static void done_cb (void *opaque,int ret) {VirtIODeviceRequest *req_data = opaque; +VirtIOPMEM *pmem = req_data->pmem;int len =iov_from_buf (req_data->elem.in_sg, req_data->elem.in_num,0 , &req_data->resp,sizeof (struct virtio_pmem_resp ));/* Callbacks are serialized, so no need to use atomic ops. */ -virtqueue_push (req_data->pmem->rq_vq, &req_data->elem, len); -virtio_notify ((VirtIODevice *)req_data->pmem, req_data->pmem->rq_vq); +virtqueue_push (pmem->rq_vq, &req_data->elem, len); +virtio_notify ((VirtIODevice *)pmem, pmem->rq_vq);trace_virtio_pmem_response ();g_free (req_data); + + pmem->inflight--; +if (!pmem->inflight) { +aio_wait_kick (); + } }static void virtio_pmem_flush (VirtIODevice *vdev,VirtQueue *vq)
@@ -85,6 +92,7 @@ static void virtio_pmem_flush(VirtIODevice *vdev, VirtQueue *vq) req_data->fd =memory_region_get_fd (&backend->mr); req_data->pmem = pmem; req_data->vdev = vdev; + pmem->inflight++;thread_pool_submit_aio (worker_cb, req_data, done_cb, req_data); }
@@ -122,6 +130,7 @@ static void virtio_pmem_realize(DeviceState *dev, Error **errp)host_memory_backend_set_mapped (pmem->memdev, true);virtio_init (vdev,VIRTIO_ID_PMEM ,sizeof (struct virtio_pmem_config )); pmem->rq_vq =virtio_add_queue (vdev,128 , virtio_pmem_flush); + pmem->inflight =1 ; }static void virtio_pmem_unrealize (DeviceState *dev)
@@ -129,6 +138,10 @@ static void virtio_pmem_unrealize(DeviceState *dev)VirtIODevice *vdev =VIRTIO_DEVICE (dev);VirtIOPMEM *pmem =VIRTIO_PMEM (dev); +/* Release the device's own reference and wait for in-flight flushes */ + pmem->inflight--; +AIO_WAIT_WHILE (NULL , pmem->inflight >0 ); +host_memory_backend_set_mapped (pmem->memdev, false);virtio_delete_queue (pmem->rq_vq);virtio_cleanup (vdev);
Signature · 1 line
MST
[PULL v2 27/30] hw/net/virtio-net: Protect from DMA re-entrancy bugs
In reply to Michael S. TsirkinFrom: Laurent Vivier <[email protected]>
Replace qemu_bh_new_guarded() by virtio_bh_new_guarded() so the bus and device use the same guard. Otherwise the DMA-reentrancy protection can be bypassed.
This update was missing in CVE-2024-3446 fix.
Fixes: CVE-2026-66022
Cc: [email protected]
Cc: Philippe Mathieu-Daudé <[email protected]>
Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4073
Message-ID: <[email protected]>
--- hw/net/virtio-net.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
hw/net/virtio-net.c +3 -2
@@ -3009,8 +3009,9 @@ static void virtio_net_add_queue(VirtIONet *n, int index) n->vqs[index].tx_vq =virtio_add_queue (vdev, n->net_conf.tx_queue_size, virtio_net_handle_tx_bh); - n->vqs[index].tx_bh =qemu_bh_new_guarded (virtio_net_tx_bh, &n->vqs[index], - &DEVICE (vdev)->mem_reentrancy_guard); + n->vqs[index].tx_bh =virtio_bh_new_guarded (DEVICE (vdev), + virtio_net_tx_bh, + &n->vqs[index]); } n->vqs[index].tx_waiting =0 ;
Signature · 1 line
MST
[PULL v2 13/30] vhost-user: assert nregions within limit
In reply to Michael S. Tsirkinscrub_shadow_regions() and vhost_user_add_remove_regions() use fixed-size stack arrays sized to VHOST_USER_MAX_RAM_SLOTS and index them with dev->mem->nregions.
nregions is calculated to never overrun these, but let's add an assert to make sure we don't get a stack overflow if there's a bug.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3910
Message-ID: <48fb8411f67e525872fb19618a886e52b670ab7f.1784896199.git.mst@redhat.com>
--- hw/virtio/vhost-user.c | 3 +++ 1 file changed, 3 insertions(+)
hw/virtio/vhost-user.c +3 -0
@@ -946,6 +946,9 @@ static int vhost_user_add_remove_regions(struct vhost_dev *dev, msg->hdr.size =sizeof (msg->payload.mem_reg); +/* Ensure nregions fits the fixed-size arrays used below. */ +assert (dev->mem->nregions <=VHOST_USER_MAX_RAM_SLOTS ); +/* Find the regions which need to be removed or added. */ scrub_shadow_regions (dev, add_reg, &nr_add_reg, rem_reg, &nr_rem_reg, shadow_pcb, track_ramblocks);
Signature · 1 line
MST
[PULL v2 08/30] virtio-scsi: fix SCSIRequest leak on a bad request
In reply to Michael S. TsirkinWhen virtio_scsi_handle_cmd_vq() cleans up prepared requests after a malformed element in the same batch, it drops only one reference even though virtio_scsi_handle_cmd_req_prepare() leaves each unsubmitted SCSIRequest with two references. This leaks the request and allows repeated bad batches to cause unbounded host memory growth.
Add a second scsi_req_unref() and clear hba_private first.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3875
Message-ID: <5092cd4716e08d29731bfe85eea82a732b837ff4.1784895264.git.mst@redhat.com>
--- include/hw/scsi/scsi.h | 1 + hw/scsi/scsi-bus.c | 7 +++++++ hw/scsi/virtio-scsi.c | 2 ++ 3 files changed, 10 insertions(+)
include/hw/scsi/scsi.h +1 -0
@@ -221,6 +221,7 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,int32_t scsi_req_enqueue (SCSIRequest *req);SCSIRequest *scsi_req_ref (SCSIRequest *req);void scsi_req_unref (SCSIRequest *req); +void scsi_req_unref_detach_hba (SCSIRequest *req);int scsi_bus_parse_cdb (SCSIDevice *dev,SCSICommand *cmd,uint8_t *buf,size_t buf_len,void *hba_private);
hw/scsi/scsi-bus.c +7 -0
@@ -1513,6 +1513,13 @@ void scsi_req_unref(SCSIRequest *req) } } +void scsi_req_unref_detach_hba (SCSIRequest *req) +{ +/* Unref when the HBA frees hba_private separately (e.g. virtio_scsi_free_req) */ + req->hba_private =NULL ; +scsi_req_unref (req); +} +/* Tell the device that we finished processing this chunk of I/O. It will start the next chunk or complete the command. */ void scsi_req_continue (SCSIRequest *req)
hw/scsi/virtio-scsi.c +2 -0
@@ -931,7 +931,9 @@ static void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) req =QTAILQ_FIRST (&reqs);QTAILQ_REMOVE (&reqs, req, next);defer_call_end (); +/* Drop both the ref from _prepare and the initial ref */ scsi_req_unref (req->sreq); +scsi_req_unref_detach_hba (req->sreq);virtqueue_detach_element (req->vq, &req->elem,0 );virtio_scsi_free_req (req); }
Signature · 1 line
MST
[PULL v2 07/30] vhost: do not crash on ring map failure
In reply to Michael S. TsirkinWhen vhost_commit() rebuilds the memory region table after a flatview change, it revalidates cached host virtual addresses for active vring parts. If a mapping is stale, QEMU abort().
This is not a security problem - only the priviledged guest can control make it invalid - but not nice e.g. for driver debugging.
Let's call virtio_error() instead, marking the device as broken.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3783
Message-ID: <71961a7dc157f552303aeea8c99a75c5e1ce904e.1784898432.git.mst@redhat.com>
--- hw/virtio/vhost.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
hw/virtio/vhost.c +3 -2
@@ -755,8 +755,9 @@ static void vhost_commit(MemoryListener *listener) (void *)(uintptr_t)dev->mem->regions[i].userspace_addr, dev->mem->regions[i].guest_phys_addr, dev->mem->regions[i].memory_size)) { -error_report ("Verify ring failure on region %d" , i); -abort (); +virtio_error (dev->vdev, +"Verify ring failure on region %d" , i); + goto out; } }
Signature · 1 line
MST
[PULL v2 19/30] virtio-snd: check rx buffer descriptor size
In reply to Michael S. TsirkinFrom: Manos Pitsidianakis <[email protected]>
It must be at least sizeof(virtio_snd_pcm_status).
I haven't verified if it's possible to get an underflow, but coverity points it out in CID 1547527 so add a check.
Message-ID: <[email protected]>
--- hw/audio/virtio-snd.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
hw/audio/virtio-snd.c +5 -3
@@ -970,12 +970,14 @@ static void virtio_snd_handle_rx_xfer(VirtIODevice *vdev, VirtQueue *vq) } stream = vsnd->pcm.streams[stream_id]; -if (stream ==NULL || stream->info.direction !=VIRTIO_SND_D_INPUT ) { + size =iov_size (elem->in_sg, elem->in_num); +if (stream ==NULL + || stream->info.direction !=VIRTIO_SND_D_INPUT + || size <sizeof (virtio_snd_pcm_status)) { goto rx_err; } + size -=sizeof (virtio_snd_pcm_status); WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { - size =iov_size (elem->in_sg, elem->in_num) - -sizeof (virtio_snd_pcm_status); buffer =g_malloc0 (sizeof (VirtIOSoundPCMBuffer) + size); buffer->elem = elem; buffer->vq = vq;
Signature · 1 line
MST
[PULL v2 20/30] virtio-snd: check for overflow before g_malloc0
In reply to Michael S. TsirkinFrom: Manos Pitsidianakis <[email protected]>
Coverity points out one g_malloc0 overflow, but it seems to be a false positive. Add a check to it regardless to fortify the code, and also add checks for every other g_malloc0 use.
Resolves: Coverity CID 1547527
Message-ID: <[email protected]>
--- hw/audio/virtio-snd.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
hw/audio/virtio-snd.c +12 -4
@@ -850,7 +850,7 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq)VirtIOSound *vsnd =VIRTIO_SND (vdev);VirtIOSoundPCMBuffer *buffer;VirtQueueElement *elem; -size_t msg_sz, size; +size_t msg_sz, size, tmp;virtio_snd_pcm_xfer hdr;uint32_t stream_id;/*
@@ -880,6 +880,8 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq)if (msg_sz !=sizeof (virtio_snd_pcm_xfer)) { goto tx_err; } +assert (iov_size (elem->out_sg, elem->out_num) >= msg_sz); + size =iov_size (elem->out_sg, elem->out_num) - msg_sz; stream_id =le32_to_cpu (hdr.stream_id); if (stream_id >= vsnd->snd_conf.streams
@@ -892,9 +894,11 @@ static void virtio_snd_handle_tx_xfer(VirtIODevice *vdev, VirtQueue *vq) goto tx_err; } +/* Check for g_malloc0 overflow. */ +if (!g_size_checked_add (&tmp,sizeof (VirtIOSoundPCMBuffer), size)) { + goto tx_err; + } WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { - size =iov_size (elem->out_sg, elem->out_num) - msg_sz; - buffer =g_malloc0 (sizeof (VirtIOSoundPCMBuffer) + size); buffer->elem = elem; buffer->populated = false;
@@ -932,7 +936,7 @@ static void virtio_snd_handle_rx_xfer(VirtIODevice *vdev, VirtQueue *vq)VirtIOSound *vsnd =VIRTIO_SND (vdev);VirtIOSoundPCMBuffer *buffer;VirtQueueElement *elem; -size_t msg_sz, size; +size_t msg_sz, size, tmp;virtio_snd_pcm_xfer hdr;uint32_t stream_id;/*
@@ -977,6 +981,10 @@ static void virtio_snd_handle_rx_xfer(VirtIODevice *vdev, VirtQueue *vq) goto rx_err; } size -=sizeof (virtio_snd_pcm_status); +/* Check for g_malloc0 overflow. */ +if (!g_size_checked_add (&tmp,sizeof (VirtIOSoundPCMBuffer), size)) { + goto rx_err; + } WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) { buffer =g_malloc0 (sizeof (VirtIOSoundPCMBuffer) + size); buffer->elem = elem;
Signature · 1 line
MST
[PULL v2 16/30] libvhost-user: fix heap overflow in vu_check_queue_inflights
In reply to Michael S. Tsirkinvu_check_queue_inflights counts inflight descriptors using inflight == 1 but copies entries using inflight != 0. If the inflight field contains an unexpected non-0/1 value, the function copies more entries than it allocates and overflows the heap buffer.
Stop the copy pass once resubmit_num reaches the counted inuse value.
Note: the value is not guest-accessible so not a security vulnerability.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3974
Message-Id: <e2315efc526c0ee918485df4be69e2b26e8b7a73.1784892981.git.mst@redhat.com>
--- subprojects/libvhost-user/libvhost-user.c | 7 +++++++ 1 file changed, 7 insertions(+)
subprojects/libvhost-user/libvhost-user.c +7 -0
@@ -1408,6 +1408,13 @@ vu_check_queue_inflights(VuDev *dev, VuVirtq *vq)for (i =0 ; i < vq->inflight->desc_num; i++) {if (vq->inflight->desc[i].inflight) { +/* +* We earlier counted exactly vq->inuse in flight - +* what is going on? +*/ +if (vq->resubmit_num >= vq->inuse) { +return -1 ; + } vq->resubmit_list[vq->resubmit_num].index = i; vq->resubmit_list[vq->resubmit_num].counter = vq->inflight->desc[i].counter;
Signature · 1 line
MST
[PULL v2 28/30] hw/virtio-rng: Fix host use-after-free (CVE-2026-50624)
In reply to Michael S. TsirkinFrom: Laurent Vivier <[email protected]>
Fix a heap-use-after-free in the virtio-rng frontend when a delayed rng-random backend completion arrives after the virtio-rng device has been hot-unplugged.
Fixes: CVE-2026-50624
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3917
Message-ID: <[email protected]>
--- include/system/rng.h | 14 ++++++++++++++ backends/rng.c | 16 ++++++++++++++++ hw/virtio/virtio-rng.c | 2 ++ 3 files changed, 32 insertions(+)
include/system/rng.h +14 -0
@@ -86,4 +86,18 @@ void rng_backend_request_entropy(RngBackend *s, size_t size,* deleted. */ void rng_backend_finalize_request (RngBackend *s,RngRequest *req); + +/** +* rng_backend_cancel_requests: +* @s: the backend that created the request +* @receive_entropy: the function invoked when entropy is available +* @opaque: data passed to @receive_entropy +* +* This function is used by the front-end to cancel all requests to a +* given backend. Requests to cancel are identified by the receive_entropy +* function and the data passed to the function. +*/ +void rng_backend_cancel_requests (RngBackend *s, +EntropyReceiveFunc *receive_entropy, +const void *opaque);#endif
backends/rng.c +16 -0
@@ -68,6 +68,22 @@ static void rng_backend_free_request(RngRequest *req)g_free (req); } +void rng_backend_cancel_requests (RngBackend *s, +EntropyReceiveFunc *receive_entropy, +const void *opaque) +{ +RngRequest *req, *next; + + QSIMPLEQ_FOREACH_SAFE(req, &s->requests, next, next) { +if (req->receive_entropy != receive_entropy || + req->opaque != opaque) { +continue ; + } +QSIMPLEQ_REMOVE (&s->requests, req, RngRequest, next); +rng_backend_free_request (req); + } +} +static void rng_backend_free_requests (RngBackend *s) {RngRequest *req, *next;
hw/virtio/virtio-rng.c +2 -0
@@ -234,6 +234,8 @@ static void virtio_rng_device_unrealize(DeviceState *dev)VirtIODevice *vdev =VIRTIO_DEVICE (dev);VirtIORNG *vrng =VIRTIO_RNG (dev); +rng_backend_cancel_requests (vrng->rng, chr_read, vrng); +qemu_del_vm_change_state_handler (vrng->vmstate);timer_free (vrng->rate_limit_timer);virtio_del_queue (vdev,0 );
Signature · 1 line
MST
[PULL v2 23/30] hw/pci-host/q35.c: Avoid early return in mch_write_config()
In reply to Michael S. TsirkinFrom: Peter Maydell <[email protected]>
In mch_write_config() we return early if has_smm_ranges is false. This is slightly bug-prone because it leaves the door open to somebody later adding non-SMM-specific code at the bottom of the function.
This case isn't as bad as the one in realize, because the function is a lot shorter. But putting the handling of the three SMM specific ranges into an if() rather than having an early return seems better.
Message-ID: <[email protected]>
--- hw/pci-host/q35.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
hw/pci-host/q35.c +12 -14
@@ -485,22 +485,20 @@ static void mch_write_config(PCIDevice *d,mch_update_pciexbar (mch); } -if (!mch->has_smm_ranges) { -return ; - } +if (mch->has_smm_ranges) { +if (ranges_overlap (address, len,MCH_HOST_BRIDGE_SMRAM , +MCH_HOST_BRIDGE_SMRAM_SIZE )) { +mch_update_smram (mch); + } -if (ranges_overlap (address, len,MCH_HOST_BRIDGE_SMRAM , -MCH_HOST_BRIDGE_SMRAM_SIZE )) { -mch_update_smram (mch); - } +if (ranges_overlap (address, len,MCH_HOST_BRIDGE_EXT_TSEG_MBYTES , +MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE )) { +mch_update_ext_tseg_mbytes (mch); + } -if (ranges_overlap (address, len,MCH_HOST_BRIDGE_EXT_TSEG_MBYTES , -MCH_HOST_BRIDGE_EXT_TSEG_MBYTES_SIZE )) { -mch_update_ext_tseg_mbytes (mch); - } - -if (ranges_overlap (address, len,MCH_HOST_BRIDGE_F_SMBASE ,1 )) { -mch_update_smbase_smram (mch); +if (ranges_overlap (address, len,MCH_HOST_BRIDGE_F_SMBASE ,1 )) { +mch_update_smbase_smram (mch); + } } }
Signature · 1 line
MST
[PULL v2 29/30] backends/rng: cap request size to avoid oversized allocation
In reply to Michael S. TsirkinFrom: Laurent Vivier <[email protected]>
rng_backend_request_entropy() uses the requested size to allocate a buffer with g_malloc(). With virtio-rng, this size comes from guest-supplied descriptor lengths. A malicious guest can set a very large descriptor length, causing QEMU to attempt a multi-gigabyte allocation and abort.
Cap the allocation to 64 KiB. The virtio-rng queue size is hardcoded to 8 entries, the EGD backend protocol limits requests to 255 bytes, the Linux kernel hwrng framework requests at most SMP_CACHE_BYTES per call (64 bytes on x86_64), and the Windows viorng driver uses a 4 KiB buffer. The worst legitimate case is 8 x 4 KiB = 32 KiB, so 64 KiB is well above any legitimate use.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3983
Message-ID: <[email protected]>
--- backends/rng.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
backends/rng.c +4 -1
@@ -11,11 +11,14 @@*/ #include "qemu/osdep.h" +#include "qemu/units.h" #include "system/rng.h" #include "qapi/error.h" #include "qemu/module.h" #include "qom/object_interfaces.h" +#define RNG_MAX_REQUEST_SIZE (64 * KiB) +void rng_backend_request_entropy (RngBackend *s,size_t size,EntropyReceiveFunc *receive_entropy,void *opaque)
@@ -27,7 +30,7 @@ void rng_backend_request_entropy(RngBackend *s, size_t size, req =g_malloc (sizeof (*req)); req->offset =0 ; - req->size = size; + req->size =MIN (size,RNG_MAX_REQUEST_SIZE ); req->receive_entropy = receive_entropy; req->opaque = opaque; req->data =g_malloc (req->size);
Signature · 1 line
MST
[PULL v2 30/30] virtio: avoid packed vring virtio_queue_empty() infinite loops (CVE-2026-16457)
In reply to Michael S. TsirkinFrom: Stefan Hajnoczi <[email protected]>
Virtqueue handler functions in device emulation code often look something like this:
while (!virtio_queue_empty(vq)) {
...pop and process virtqueue element...
}virtio-blk, virtio-scsi, virtio-crypto, and vhost-shadow-virtqueue use this pattern.
The device may break (i.e. hit an error that requires device reset) during the loop. virtio_queue_empty() returns 1 for broken split vrings but not for broken packed vrings, leading to an infinite loop.
Adjust the packed vring behavior to match split vrings and avoid infinite loops.
Fixes: CVE-2026-16457
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3968
Message-ID: <[email protected]>
--- hw/virtio/virtio.c | 4 ++++ 1 file changed, 4 insertions(+)
hw/virtio/virtio.c +4 -0
@@ -763,6 +763,10 @@ static int virtio_queue_packed_empty_rcu(VirtQueue *vq)struct VRingPackedDesc desc;VRingMemoryRegionCaches *cache; +if (virtio_device_disabled (vq->vdev)) { +return 1 ; + } +if (unlikely (!vq->vring.desc)) {return 1 ; }
Signature · 1 line
MST
[PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
In reply to Michael S. Tsirkinvirtio-mmio reports VIRTQUEUE_MAX_SIZE (1024) as QUEUE_NUM_MAX for every queue, regardless of the size the device passes to virtio_add_queue().
This works by accident because QEMU mostly does not care about the ring size - the guest is the one allocating memory here. But this changes with in-order vqs where qemu is the one allocating resources. Now, specifying a larger vq than allocated causes an OOB memory access.
To fix:
- for new machine types, report the actual max queue size to guest
- for old machine types, use a compat property to allocate 1k sized queues
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3882
Cc: Peter Maydell <[email protected]>
Message-ID: <8715acbb9516e67e2a776cda6f9edf105343f788.1784930765.git.mst@redhat.com>
--- include/hw/virtio/virtio-bus.h | 1 + include/hw/virtio/virtio-mmio.h | 1 + hw/core/machine.c | 1 + hw/virtio/virtio-mmio.c | 7 +++---- hw/virtio/virtio.c | 11 +++++++++++ 5 files changed, 17 insertions(+), 4 deletions(-)
include/hw/virtio/virtio-bus.h +1 -0
@@ -30,6 +30,7 @@#include "qom/object.h" #define TYPE_VIRTIO_BUS "virtio-bus" +#define VIRTIO_QUEUE_SIZE_OVERRIDE "x-override-queue-size"typedef struct VirtioBusClass VirtioBusClass ;typedef struct VirtioBusState VirtioBusState ; DECLARE_OBJ_CHECKERS(VirtioBusState, VirtioBusClass,
include/hw/virtio/virtio-mmio.h +1 -0
@@ -69,6 +69,7 @@ struct VirtIOMMIOProxy {/* Fields only used for non-legacy (v2) devices */ uint32_t guest_features[2 ];VirtIOMMIOQueue vqs[VIRTIO_QUEUE_MAX ]; + uint16_t override_queue_size; };#endif
hw/core/machine.c +1 -0
@@ -41,6 +41,7 @@#include "hw/arm/smmuv3.h" GlobalProperty hw_compat_11_0[] = { + {"virtio-mmio" ,VIRTIO_QUEUE_SIZE_OVERRIDE ,"1024" }, {"chardev-vc" ,"encoding" ,"cp437" }, {"tpm-crb" ,"cap-chunk" ,"off" }, {"tpm-crb" ,"x-allow-chunk-migration" ,"off" },
hw/virtio/virtio-mmio.c +3 -4
@@ -172,10 +172,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size) >> (32 * proxy->host_features_sel); }case VIRTIO_MMIO_QUEUE_NUM_MAX : -if (!virtio_queue_get_num (vdev, vdev->queue_sel)) { -return 0 ; - } -return VIRTQUEUE_MAX_SIZE ; +return virtio_queue_get_max_num (vdev, vdev->queue_sel);case VIRTIO_MMIO_QUEUE_PFN :if (!proxy->legacy) { qemu_log_mask(LOG_GUEST_ERROR,
@@ -738,6 +735,8 @@ static const Property virtio_mmio_properties[] = {DEFINE_PROP_BOOL ("force-legacy" , VirtIOMMIOProxy, legacy, true),DEFINE_PROP_BIT ("ioeventfd" , VirtIOMMIOProxy, flags,VIRTIO_IOMMIO_FLAG_USE_IOEVENTFD_BIT , true), +DEFINE_PROP_UINT16 (VIRTIO_QUEUE_SIZE_OVERRIDE , VirtIOMMIOProxy, + override_queue_size, 0), };static void virtio_mmio_realizefn (DeviceState *d,Error **errp)
hw/virtio/virtio.c +11 -0
@@ -2568,6 +2568,17 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,if (i ==VIRTIO_QUEUE_MAX || queue_size >VIRTQUEUE_MAX_SIZE )abort (); +BusState *qbus =qdev_get_parent_bus (DEVICE (vdev)); +if (qbus && qbus->parent && +object_property_find (OBJECT (qbus->parent),VIRTIO_QUEUE_SIZE_OVERRIDE )) { +int override =object_property_get_int (OBJECT (qbus->parent), +VIRTIO_QUEUE_SIZE_OVERRIDE , + &error_abort); +if (override) { + queue_size = override; + } + } + vdev->vq[i].vring.num = queue_size; vdev->vq[i].vring.num_default = queue_size; vdev->vq[i].vring.align =VIRTIO_PCI_VRING_ALIGN ;
Signature · 1 line
MST
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 8 lines · Michael S. Tsirkin
virtio-mmio reports
VIRTQUEUE_MAX_SIZE(1024) asQUEUE_NUM_MAXfor every queue, regardless of the size the device passes tovirtio_add_queue().This works by accident because QEMU mostly does not care about the ring size - the guest is the one allocating memory here. But this changes with in-order vqs where qemu is the one allocating resources. Now, specifying a larger vq than allocated causes an OOB memory access.
Quoted 6 lines
To fix:
- for new machine types, report the actual max queue size to guest
- for old machine types, use a compat property to allocate 1k sized queuesFixes 525d82e323 ("virtio: fix queue size validation against allocated maximum")
This is actually d530f2dfbd2 in the master branch. But his is an interesting case of the Fixes: tag.
The change it is fixing (this non-existing commit) is in the same pull request and comes right NEXT to this fix! So we fix something first and introduce it.. later? :)
It's a fun stuff.
But besides this, I wonder what can we do with that for the stable series...
Quoted 7 lines
Fixes: CVE-2026-50626
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3882
Cc: Peter Maydell <[email protected]>
Message-ID: <8715acbb9516e67e2a776cda6f9edf105343f788.1784930765.git.mst@redhat.com>Acked-by Yonggang Luo <[email protected]>Reported-by Miku Hatsune <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>
...
hw/core/machine.c line 44
Quoted 9 lines
hw/core/machine.c +1
-0@@ -41,6 +41,7 @@#include "hw/arm/smmuv3.h" GlobalProperty hw_compat_11_0[] = { + {"virtio-mmio" ,VIRTIO_QUEUE_SIZE_OVERRIDE ,"1024" },
..because it is adding a new field into the migration stream, which is a problem for stable series.
Or is it a problem in previous releases?
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 16 lines · Michael Tokarev
Quoted 8 lines · Michael S. Tsirkin
virtio-mmio reports
VIRTQUEUE_MAX_SIZE(1024) asQUEUE_NUM_MAXfor every queue, regardless of the size the device passes tovirtio_add_queue().This works by accident because QEMU mostly does not care about the ring size - the guest is the one allocating memory here. But this changes with in-order vqs where qemu is the one allocating resources. Now, specifying a larger vq than allocated causes an OOB memory access.
Quoted 6 lines
To fix:
- for new machine types, report the actual max queue size to guest
- for old machine types, use a compat property to allocate 1k sized queuesFixes 525d82e323 ("virtio: fix queue size validation against allocated maximum")
Quoted 8 lines
This is actually
d530f2dfbd2in the master branch. But his is an interesting case of the Fixes: tag.The change it is fixing (this non-existing commit) is in the same pull request and comes right NEXT to this fix! So we fix something first and introduce it.. later? :)
It's a fun stuff.
Got some wires crossed here) thanks for noticing.
Quoted 11 lines
But besides this, I wonder what can we do with that for the stable series...
Quoted 7 lines
Fixes: CVE-2026-50626
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3882
Cc: Peter Maydell <[email protected]>
Message-ID: <8715acbb9516e67e2a776cda6f9edf105343f788.1784930765.git.mst@redhat.com>Acked-by Yonggang Luo <[email protected]>Reported-by Miku Hatsune <[email protected]>Signed-off-by Michael S. Tsirkin <[email protected]>...
Quoted 11 lines
hw/core/machine.c line 43
Quoted 8 lines
hw/core/machine.c +1
-0@@ -41,6 +41,7 @@#include "hw/arm/smmuv3.h" GlobalProperty hw_compat_11_0[] = { + {"virtio-mmio" ,VIRTIO_QUEUE_SIZE_OVERRIDE ,"1024" },..because it is adding a new field into the migration stream, which is a problem for stable series.
this is compat not a migration stream, right? For stable, the value needs to be 1024 for all types.
Quoted 5 lines
Or is it a problem in previous releases?
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:40:24AM +0300, Michael Tokarev wrote:
...
Quoted 13 lines
Quoted 11 lines
hw/core/machine.c line 43
Quoted 8 lines
hw/core/machine.c +1
-0@@ -41,6 +41,7 @@#include "hw/arm/smmuv3.h" GlobalProperty hw_compat_11_0[] = { + {"virtio-mmio" ,VIRTIO_QUEUE_SIZE_OVERRIDE ,"1024" },..because it is adding a new field into the migration stream, which is a problem for stable series.
Quoted 1 line
this is compat not a migration stream, right?
Aha, yes, I was a bit tired today.
Quoted 1 line
For stable, the value needs to be 1024 for all types.
So this whole change, for older versions, becomes just:
hw/virtio/virtio.c +2 -0
@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,if (i ==VIRTIO_QUEUE_MAX || queue_size >VIRTQUEUE_MAX_SIZE )abort (); + queue_size =VIRTQUEUE_MAX_SIZE ;/* fixed properly in 11.1. */ + vdev->vq[i].vring.num = queue_size; vdev->vq[i].vring.num_default = queue_size; vdev->vq[i].vring.align =VIRTIO_PCI_VRING_ALIGN ;
so we override supplied size just like when x-override-queue-size is set for compat-11.0 or earlier.
Got it.
Now, the fun continues - this is the first time in qemu history when a bigger change reduces in stable series into such a one-liner :)
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 28 lines · Michael Tokarev
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:40:24AM +0300, Michael Tokarev wrote:
...
Quoted 13 lines
Quoted 11 lines
hw/core/machine.c line 43
Quoted 8 lines
hw/core/machine.c +1
-0@@ -41,6 +41,7 @@#include "hw/arm/smmuv3.h" GlobalProperty hw_compat_11_0[] = { + {"virtio-mmio" ,VIRTIO_QUEUE_SIZE_OVERRIDE ,"1024" },..because it is adding a new field into the migration stream, which is a problem for stable series.
Quoted 1 line
this is compat not a migration stream, right?
Aha, yes, I was a bit tired today.
Quoted 1 line
For stable, the value needs to be 1024 for all types.
So this whole change, for older versions, becomes just:
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2576
Quoted 5 lines
hw/virtio/virtio.c +2
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,if (i ==VIRTIO_QUEUE_MAX || queue_size >VIRTQUEUE_MAX_SIZE )abort (); + queue_size =VIRTQUEUE_MAX_SIZE ;/* fixed properly in 11.1. */ +
Or you can stick to upstream and add the compat entry for 11.0.
Quoted 16 lines
hw/virtio/virtio.c +0
-0vdev->vq[i].vring.num = queue_size; vdev->vq[i].vring.num_default = queue_size; vdev->vq[i].vring.align =VIRTIO_PCI_VRING_ALIGN ;so we override supplied size just like when x-override-queue-size is set for compat-11.0 or earlier.
Got it.
Now, the fun continues - this is the first time in qemu history when a bigger change reduces in stable series into such a one-liner :)
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:17:34PM +0300, Michael Tokarev wrote:
..
Quoted 8 lines
Quoted 8 lines
So this whole change, for older versions, becomes just:
hw/virtio/virtio.c +0
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,if (i ==VIRTIO_QUEUE_MAX || queue_size >VIRTQUEUE_MAX_SIZE )abort ();
Quoted 4 lines
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2576
Quoted 2 lines
hw/virtio/virtio.c +2
-0+ queue_size =VIRTQUEUE_MAX_SIZE ;/* fixed properly in 11.1. */ +Or you can stick to upstream and add the compat entry for 11.0.
Compat entry for 11.0 which will never be used?
With a non-obvious logic which boils down to this single line?
Or what do you mean?
BTW, can't this logic be implemented without a compat property in the first place? This x-override-queue-size seems like a hack. or a.. twisted way to compare a machine version.
Can't this very place become
if (machine_version < 11.1) {
queue_size = VIRTQUEUE_MAX_SIZE;
}instead of using this indirect property?
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 5 lines · Michael Tokarev
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:17:34PM +0300, Michael Tokarev wrote:
..
Quoted 2 lines
Quoted 2 lines
So this whole change, for older versions, becomes just:
Quoted 7 lines
Quoted 7 lines
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2575
Quoted 7 lines
hw/virtio/virtio.c +1
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, if (i == VIRTIO_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE) abort(); + queue_size = VIRTQUEUE_MAX_SIZE; /* fixed properly in 11.1. */
BTW, with this override in place, the second half of this fix isn't even needed - for older/stable versions, this line is enough to fix CVE-2026-50626.
Quoted 3 lines
BTW, can't this logic be implemented without a compat property in the first place? This x-override-queue-size seems like a hack. or a.. twisted way to compare a machine version.
Besides being a hack, it also redefines a #define within a compat property (using a "magic number" instead of a symbolic constant) in a non-obvious way. Though it might be a good way here, since the property value is the actual historical number which should never be changed.
Quoted 7 lines
Can't this very place become
if (
machine_version< 11.1) {
queue_size=VIRTQUEUE_MAX_SIZE;
}instead of using this indirect property?
That would be great if this patch can be reduced to this (plus the fixup for VIRTIO_MMIO_QUEUE_NUM_MAX in virtio_mmio_read) without (ab)using properties like this.
BTW, isn't x-override-queue-size becoming a part of user interface?
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 33 lines · Michael Tokarev
Quoted 5 lines · Michael Tokarev
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:17:34PM +0300, Michael Tokarev wrote:
..
Quoted 2 lines
Quoted 2 lines
So this whole change, for older versions, becomes just:
Quoted 7 lines
Quoted 7 lines
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2575
Quoted 7 lines
hw/virtio/virtio.c +1
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, if (i == VIRTIO_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE) abort(); + queue_size = VIRTQUEUE_MAX_SIZE; /* fixed properly in 11.1. */BTW, with this override in place, the second half of this fix isn't even needed - for older/stable versions, this line is enough to fix CVE-2026-50626.
Quoted 3 lines
BTW, can't this logic be implemented without a compat property in the first place? This x-override-queue-size seems like a hack. or a.. twisted way to compare a machine version.
Besides being a hack, it also redefines a #define within a compat property (using a "magic number" instead of a symbolic constant) in a non-obvious way. Though it might be a good way here, since the property value is the actual historical number which should never be changed.
Quoted 6 lines
Can't this very place become
if (
machine_version< 11.1) {
queue_size=VIRTQUEUE_MAX_SIZE;
}
Quoted 7 lines
Quoted 1 line
instead of using this indirect property?
That would be great if this patch can be reduced to this (plus the fixup for
VIRTIO_MMIO_QUEUE_NUM_MAXinvirtio_mmio_read) without (ab)using properties like this.BTW, isn't x-override-queue-size becoming a part of user interface?
I guess you dislike how qemu handles compatibility generally?
Quoted 3 lines
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
In reply to Michael TokarevQuoted 25 lines · Michael Tokarev
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:17:34PM +0300, Michael Tokarev wrote:
..
Quoted 8 lines
Quoted 8 lines
So this whole change, for older versions, becomes just:
hw/virtio/virtio.c +0
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, if (i == VIRTIO_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE) abort();Quoted 4 lines
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2576
Quoted 2 lines
hw/virtio/virtio.c +2
-0+ queue_size = VIRTQUEUE_MAX_SIZE; /* fixed properly in 11.1. */ +Or you can stick to upstream and add the compat entry for 11.0.
Compat entry for 11.0 which will never be used?
With a non-obvious logic which boils down to this single line?Or what do you mean?
BTW, can't this logic be implemented without a compat property in the first place? This x-override-queue-size seems like a hack. or a.. twisted way to compare a machine version.
Quoted 7 lines
Can't this very place become
if (
machine_version< 11.1) {
queue_size=VIRTQUEUE_MAX_SIZE;
}instead of using this indirect property?
Michael, I still don't understand what we're doing here and - now - why do we need to distinguish between old and new machine types at all.
Behavior before the fixes:
qemu always reported queue size to be the maximum allowed one (1024),
even if actual queue size requested and allocated was smaller
qemu accepted requests to elements past the allocated size, which
allowed to read/write past the allocated buffer - which can't be considered part of ABI, obviously :)
Behavior after the fixes, for new machine types:
qemu reports actually requested and allocated queue size qemu properly disallows access past the allocated queue size
What would break if we switch to the new behavior unconditionally, regardless of the machine types?
I don't understand why do you suggest to implement the same logic for older/stable qemu versions if it will always evaluate to allocating the max size for the queue regardless of the requested size.
Thanks,
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 45 lines · Michael Tokarev
Quoted 25 lines · Michael Tokarev
Quoted 1 line · Michael S. Tsirkin
On Tue, Jul 28, 2026 at 11:17:34PM +0300, Michael Tokarev wrote:
..
Quoted 8 lines
Quoted 8 lines
So this whole change, for older versions, becomes just:
hw/virtio/virtio.c +0
-0@@ -2572,6 +2572,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, if (i == VIRTIO_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE) abort();Quoted 4 lines
hw/virtio/virtio.c VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, · line 2576
Quoted 2 lines
hw/virtio/virtio.c +2
-0+ queue_size = VIRTQUEUE_MAX_SIZE; /* fixed properly in 11.1. */ +Or you can stick to upstream and add the compat entry for 11.0.
Compat entry for 11.0 which will never be used?
With a non-obvious logic which boils down to this single line?Or what do you mean?
BTW, can't this logic be implemented without a compat property in the first place? This x-override-queue-size seems like a hack. or a.. twisted way to compare a machine version.
Quoted 7 lines
Can't this very place become
if (
machine_version< 11.1) {
queue_size=VIRTQUEUE_MAX_SIZE;
}instead of using this indirect property?
Michael, I still don't understand what we're doing here and - now - why do we need to distinguish between old and new machine types at all.
Behavior before the fixes:
qemu always reported queue size to be the maximum allowed one (1024),
even if actual queue size requested and allocated was smaller
qemu accepted requests to elements past the allocated size, which
allowed to read/write past the allocated buffer - which can't be considered part of ABI, obviously :)
Quoted 7 lines
Behavior after the fixes, for new machine types:
qemu reports actually requested and allocated queue size qemu properly disallows access past the allocated queue size
What would break if we switch to the new behavior unconditionally, regardless of the machine types?
It's guest visible. You will get two reads from same register suddenly returning different values.
Quoted 7 lines
I don't understand why do you suggest to implement the same logic for older/stable qemu versions if it will always evaluate to allocating the max size for the queue regardless of the requested size.
Thanks,
/mjt
My preference, normally, is to just stick to upstream as much as possibly. I just do not want slightly different code bases when we can trivially have one. If nothing else, less of a chance a follow up patch will cause conflicts, and then it snowballs from there.
Signature · 1 line
MST
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 7 lines · Michael S. Tsirkin
Quoted 2 lines · Michael Tokarev
What would break if we switch to the new behavior unconditionally, regardless of the machine types?
It's guest visible. You will get two reads from same register suddenly returning different values.
The old value basically makes no sense. We returned a wrong value here, now we return the correct one. It's like, I don't know, a lot of fixes in various other areas - when it was a bug in qemu impl. of some register read which was subsequently corrected - it doesn't need a compat property to continue returning a wrong/bogus value for old machines? I think here, there's no real difference - because the guest can't access the "extra" space anyway without causing some unexpected results (usually a crash).
Quoted 4 lines
Quoted 4 lines
I don't understand why do you suggest to implement the same logic for older/stable qemu versions if it will always evaluate to allocating the max size for the queue regardless of the requested size.
Quoted 8 lines
Quoted 3 lines
Thanks,
/mjt
My preference, normally, is to just stick to upstream as much as possibly. I just do not want slightly different code bases when we can trivially have one. If nothing else, less of a chance a follow up patch will cause conflicts, and then it snowballs from there.
Yes, this is exactly my preference as well. It is more, I often pick up some other changes to stable - changes which aren't fixing anything, - say, some renames or code shuffling around - just to make the resulting code closer to the master branch, so that subsequent changes has much more chances to apply cleanly.
In this case I can drop the addition to hw_compat_11_0[] (because it doesn't exist in 11.0 and before) and keep everything else. There will be quite some code which boils down to a single line (queue_size = QUEUE_SIZE_MAX). Twisted code which does not contain the main trigger (the property) at all. This change and the subsequent change which it fixes - in reality, all these ifs and conditions would be just useless, but will give someone a puzzle to solve, wtf is going on here?
Ofc it would be ideal - from the back-porting PoV only - to just drop this all entirely and unconditionally report the correct queue size to begin with, as per above.
Hwell.. :)
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 11 lines · Michael Tokarev
Quoted 7 lines · Michael S. Tsirkin
Quoted 2 lines · Michael Tokarev
What would break if we switch to the new behavior unconditionally, regardless of the machine types?
It's guest visible. You will get two reads from same register suddenly returning different values.
The old value basically makes no sense. We returned a wrong value here, now we return the correct one.
So now, guest read the big value, and it is writing that value back. But since we changed the value to smaller one now guest is writing a value bigger than the max and out of spec and it will fail. Not nice at all.
Quoted 5 lines
It's like, I don't know, a lot
of fixes in various other areas - when it was a bug in qemu impl. of some register read which was subsequently corrected - it doesn't need a compat property to continue returning a wrong/bogus value for old machines?
Yes you will find we do that a lot.
See for example x-pci-express-writeable-slt-bug there were many more we just dropped support for the affected machine types.
Quoted 3 lines
I think here, there's no real difference - because the
guest can't access the "extra" space anyway without causing some unexpected results (usually a crash).
Of course if it was like this we would have noticed much earlier. FYI most guests configure exactly the size we specify as max. The state is in guest memory, so it just works.
For the typical scenario (no in-order), and no attempts to fill all of the queue with a single request, things simply work with a slightly bigger queue. Maybe a bit higher memory requirement, that is all.
I'm not strongly objecting to changing the original patch. This needs a bunch of thought though, as we are breaking a fundamental promise of live migration.
And given we are in freeze and it's a CVE, there's some urgency to get the fix merged.
Quoted 19 lines
Quoted 4 lines
Quoted 4 lines
I don't understand why do you suggest to implement the same logic for older/stable qemu versions if it will always evaluate to allocating the max size for the queue regardless of the requested size.
Quoted 8 lines
Quoted 3 lines
Thanks,
/mjt
My preference, normally, is to just stick to upstream as much as possibly. I just do not want slightly different code bases when we can trivially have one. If nothing else, less of a chance a follow up patch will cause conflicts, and then it snowballs from there.
Yes, this is exactly my preference as well. It is more, I often pick up some other changes to stable - changes which aren't fixing anything, - say, some renames or code shuffling around - just to make the resulting code closer to the master branch, so that subsequent changes has much more chances to apply cleanly.
Quoted 8 lines
In this case I can drop the addition to hw_compat_11_0[] (because it doesn't exist in 11.0 and before) and keep everything else. There will be quite some code which boils down to a single line (
queue_size=QUEUE_SIZE_MAX). Twisted code which does not contain the main trigger (the property) at all. This change and the subsequent change which it fixes - in reality, all these ifs and conditions would be just useless, but will give someone a puzzle to solve, wtf is going on here?
They just need to look at the upstream commit and it's clear.
My suggestion would be to propose a cleanup upstream, including an analysis of the risks, and we'll discuss.
Or you can do original development in the stable branch if you prefer, be my guest, but don't expect it to get same level of scrutiny as upstream code gets. If anything breaks, it's on you.
Quoted 7 lines
Ofc it would be ideal - from the back-porting PoV only - to just drop this all entirely and unconditionally report the correct queue size to begin with, as per above.
Hwell.. :)
/mjt
A discussion for upstream, imho.
Signature · 1 line
MST
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 9 lines · Michael S. Tsirkin
Quoted 2 lines · Michael Tokarev
The old value basically makes no sense. We returned a wrong value here, now we return the correct one.
So now, guest read the big value, and it is writing that value back. But since we changed the value to smaller one now guest is writing a value bigger than the max and out of spec and it will fail. Not nice at all.
Just to clarify: this might happen only after migration (to qemu "fixed" the way I was thinking).
[]
Quoted 5 lines
I'm not strongly objecting to changing the original patch. This needs a bunch of thought though, as we are breaking a fundamental promise of live migration.
And given we are in freeze and it's a CVE, there's some urgency to get the fix merged.
Yes, I fully understand the time constraints we have here. Also, it's fixed for 11.1+, and the release is coming - and once it's out, this whole problem is history already :)
[...]
Quoted 6 lines
My suggestion would be to propose a cleanup upstream, including an analysis of the risks, and we'll discuss.
Or you can do original development in the stable branch if you prefer, be my guest, but don't expect it to get same level of scrutiny as upstream code gets. If anything breaks, it's on you.
There's no need for a stable-only solution which is different in logic, since it is the upstream version with current logic which we must be able to migrate to anyway. So any changes needs to be in agreement with the upstream.
Quoted 5 lines
Quoted 3 lines
Ofc it would be ideal - from the back-porting PoV only - to just drop this all entirely and unconditionally report the correct queue size to begin with, as per above.
A discussion for upstream, imho.
Yes, this is exactly what I'm doing - discussing it here. But without a proposal yet ;))
Ok.
For now, for the two active stable series, I'll pick up only this change out of the two, and even it reduced to a one-liner I mentioned earlier (doing queue_size = QUEUE_SIZE_MAX). This is sufficient to fix the bug in stable series (the other change isn't needed). If anything will come touching this place (including a more elegant solution), it's trivial to revert it and re-do, no snow-ball effect is expected.
And thank you for your patience!
/mjt
Re: [PULL v2 09/30] virtio-mmio: fix QUEUE_NUM_MAX
Quoted 6 lines · Michael Tokarev
Quoted 5 lines · Michael S. Tsirkin
Quoted 2 lines · Michael Tokarev
The old value basically makes no sense. We returned a wrong value here, now we return the correct one.
Quoted 7 lines
Quoted 4 lines
So now, guest read the big value, and it is writing that value back. But since we changed the value to smaller one now guest is writing a value bigger than the max and out of spec and it will fail. Not nice at all.
Just to clarify: this might happen only after migration (to qemu "fixed" the way I was thinking).
read num before migration, write after migration.
Quoted 47 lines
[]
Quoted 5 lines
I'm not strongly objecting to changing the original patch. This needs a bunch of thought though, as we are breaking a fundamental promise of live migration.
And given we are in freeze and it's a CVE, there's some urgency to get the fix merged.Yes, I fully understand the time constraints we have here. Also, it's fixed for 11.1+, and the release is coming - and once it's out, this whole problem is history already :)
[...]
Quoted 6 lines
My suggestion would be to propose a cleanup upstream, including an analysis of the risks, and we'll discuss.
Or you can do original development in the stable branch if you prefer, be my guest, but don't expect it to get same level of scrutiny as upstream code gets. If anything breaks, it's on you.
There's no need for a stable-only solution which is different in logic, since it is the upstream version with current logic which we must be able to migrate to anyway. So any changes needs to be in agreement with the upstream.
Quoted 5 lines
Quoted 3 lines
Ofc it would be ideal - from the back-porting PoV only - to just drop this all entirely and unconditionally report the correct queue size to begin with, as per above.
A discussion for upstream, imho.
Yes, this is exactly what I'm doing - discussing it here. But without a proposal yet ;))
Ok.
For now, for the two active stable series, I'll pick up only this change out of the two, and even it reduced to a one-liner I mentioned earlier (doing
queue_size = QUEUE_SIZE_MAX). This is sufficient to fix the bug in stable series (the other change isn't needed). If anything will come touching this place (including a more elegant solution), it's trivial to revert it and re-do, no snow-ball effect is expected.And thank you for your patience!
/mjt
[PULL v2 15/30] libvhost-user: validate last_batch_head in vu_check_queue_inflights
In reply to Michael S. Tsirkinvu_check_queue_inflights uses last_batch_head from the frontend-controlled inflight shared memory as an index into desc[] without bounds checking. A malicious or buggy frontend can set last_batch_head >= desc_num, causing an out-of-bounds write.
Validate last_batch_head before using it.
Note: the value is not guest-accessible so not a security vulnerability.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3974
Message-Id: <8133770c75c9907578dd551f4d468140a0a75cd2.1784892981.git.mst@redhat.com>
--- subprojects/libvhost-user/libvhost-user.c | 6 ++++++ 1 file changed, 6 insertions(+)
subprojects/libvhost-user/libvhost-user.c +6 -0
@@ -1379,6 +1379,12 @@ vu_check_queue_inflights(VuDev *dev, VuVirtq *vq) vq->counter =0 ;if (unlikely (vq->inflight->used_idx != vq->used_idx)) { +if (vq->inflight->last_batch_head >= vq->inflight->desc_num) { +vu_panic (dev,"vu_check_queue_inflights: last_batch_head %u " +"out of range (desc_num %u)" , + vq->inflight->last_batch_head, vq->inflight->desc_num); +return -1 ; + } vq->inflight->desc[vq->inflight->last_batch_head].inflight =0 ;barrier ();
Signature · 1 line
MST
Stefan Hajnoczi
AppliedApplied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/11.1 for any user-visible changes.