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?
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;
}
