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" <[email protected]>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3863
> > Signed-off-by: Maxim Levitsky <[email protected]>
> > ---
> > 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;
> > }
