accel/kvm/kvm-all.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
KVM's coalesced mmio ring buffer has entries that contain deffered MMIO
writes.
Each such entry contains the length, an 8 byte value and a guest address.
While the length value comes from the kernel, it is still possible for
an attacker to corrupt the length field using another exploit.
The attacker can then invoke kvm_flush_coalesced_mmio_buffer, which blindly
trusts the length and can be used to inflict further damage.
Add a sanity check on the length field to prevent this.
This patch was only compile tested.
Reported by: "Labs, STAR" <info@starlabs.sg>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3863
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
accel/kvm/kvm-all.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 83cbd120a847..82898fd89148 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3174,8 +3174,15 @@ void kvm_flush_coalesced_mmio_buffer(void)
ent = &ring->coalesced_mmio[ring->first];
as = ent->pio == 1 ? &address_space_io : &address_space_memory;
- address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
- ent->data, ent->len);
+
+ if (ent->len > sizeof(ent->data)) {
+ warn_report("coalesced MMIO entry has invalid len %u",
+ ent->len);
+ } else {
+ address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
+ ent->data, ent->len);
+ }
+
smp_wmb();
ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
}
--
2.54.0Queued, thanks. I changed the code to what Philippe suggested:
ent = &ring->coalesced_mmio[ring->first];
if (ent->len > sizeof(ent->data)) {
warn_report("coalesced MMIO entry has invalid len %u",
ent->len);
} else {
as = ent->pio == 1 ? &address_space_io : &address_space_memory;
address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
ent->data, ent->len);
}
Paolo
On 23/9/26 18:46, Paolo Bonzini wrote:
> Queued, thanks. I changed the code to what Philippe suggested:
>
> ent = &ring->coalesced_mmio[ring->first];
> if (ent->len > sizeof(ent->data)) {
> warn_report("coalesced MMIO entry has invalid len %u",
> ent->len);
> } else {
> as = ent->pio == 1 ? &address_space_io : &address_space_memory;
> address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
> ent->data, ent->len);
> }
Thank you!
On 24/9/26 06:36, Philippe Mathieu-Daudé wrote: > On 23/9/26 18:46, Paolo Bonzini wrote: >> Queued, thanks. Maybe worth adding (also to the few other Maxim sent): Cc: qemu-stable@nongnu.org
Hi Maxim,
On 23/9/26 02:46, Maxim Levitsky wrote:
> KVM's coalesced mmio ring buffer has entries that contain deffered MMIO
> writes.
>
> Each such entry contains the length, an 8 byte value and a guest address.
>
> While the length value comes from the kernel, it is still possible for
> an attacker to corrupt the length field using another exploit.
>
> The attacker can then invoke kvm_flush_coalesced_mmio_buffer, which blindly
> trusts the length and can be used to inflict further damage.
>
> Add a sanity check on the length field to prevent this.
>
> This patch was only compile tested.
>
> Reported by: "Labs, STAR" <info@starlabs.sg>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3863
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
> accel/kvm/kvm-all.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 83cbd120a847..82898fd89148 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3174,8 +3174,15 @@ void kvm_flush_coalesced_mmio_buffer(void)
>
> ent = &ring->coalesced_mmio[ring->first];
Could we move the check here?
Do we want to keep processing the other entries if the current %len
is broken or break and return?
> as = ent->pio == 1 ? &address_space_io : &address_space_memory;
> - address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
> - ent->data, ent->len);
> +
> + if (ent->len > sizeof(ent->data)) {
> + warn_report("coalesced MMIO entry has invalid len %u",
> + ent->len);
> + } else {
> + address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
> + ent->data, ent->len);
> + }
> +
> smp_wmb();
> ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
> }
On Wed, 2026-09-23 at 07:07 +0200, Philippe Mathieu-Daudé wrote:
> Hi Maxim,
>
> On 23/9/26 02:46, Maxim Levitsky wrote:
> > KVM's coalesced mmio ring buffer has entries that contain deffered MMIO
> > writes.
> >
> > Each such entry contains the length, an 8 byte value and a guest address.
> >
> > While the length value comes from the kernel, it is still possible for
> > an attacker to corrupt the length field using another exploit.
> >
> > The attacker can then invoke kvm_flush_coalesced_mmio_buffer, which blindly
> > trusts the length and can be used to inflict further damage.
> >
> > Add a sanity check on the length field to prevent this.
> >
> > This patch was only compile tested.
> >
> > Reported by: "Labs, STAR" <info@starlabs.sg>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3863
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > accel/kvm/kvm-all.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 83cbd120a847..82898fd89148 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -3174,8 +3174,15 @@ void kvm_flush_coalesced_mmio_buffer(void)
> >
> > ent = &ring->coalesced_mmio[ring->first];
>
> Could we move the check here?
Yes.
>
> Do we want to keep processing the other entries if the current %len
> is broken or break and return?
Honestly this is such a theoretical situation that I don't know what is better here.
One could say that skipping non corrupted writes also gives some advantage to the attacker.
This patch goal is mostly to silence the static analysis tools (read: AI),
because technically this is a buffer overflow.
Best regards,
Maxim Levitsky
>
> > as = ent->pio == 1 ? &address_space_io : &address_space_memory;
> > - address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
> > - ent->data, ent->len);
> > +
> > + if (ent->len > sizeof(ent->data)) {
> > + warn_report("coalesced MMIO entry has invalid len %u",
> > + ent->len);
> > + } else {
> > + address_space_write(as, ent->phys_addr, MEMTXATTRS_UNSPECIFIED,
> > + ent->data, ent->len);
> > + }
> > +
> > smp_wmb();
> > ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
> > }
© 2016 - 2026 Red Hat, Inc.