[PATCH v2] KVM: coalesced_mmio: reject zone sizes that don't fit in an int

[PATCH v2] KVM: coalesced_mmio: reject zone sizes that don't fit in an int

[Hang Nan]

kvm_vm_ioctl_register_coalesced_mmio() validates @zone->pio but not
@zone->size.  The u32 size is handed to kvm_io_bus_register_dev() as an
"int len", so any size >= 0x80000000 silently becomes negative.

coalesced_mmio_in_range() bails out immediately for len < 0, so such a
zone never matches its own range:

  - coalesced MMIO writes for the zone are no longer coalesced and are
    forwarded to userspace instead;
  - KVM_UNREGISTER_COALESCED_MMIO returns 0 but does not remove the zone,
    because the unregister path uses coalesced_mmio_in_range() for matching
    too, leaving the kvm_io_device permanently registered;
  - the leaked device occupies a slot on the MMIO/PIO bus.  After
    NR_IOBUS_DEVS - 1 (998) such zones every further
    kvm_io_bus_register_dev() on that bus fails with -ENOSPC, including
    ioeventfds and valid coalesced zones, effectively killing the VM's
    device model until the VM is destroyed.

A negative len additionally breaks the sorted-interval invariant used by
kvm_io_bus_cmp(), which computes the end of an interval as "addr + len".

ioeventfd rejects lengths outside {0, 1, 2, 4, 8}, so the same ioctl
family cannot be abused this way; coalesced MMIO has no equivalent check.

Validate the zone in kvm_vm_ioctl_unregister_coalesced_mmio() as well: it
has the same gap today and is only saved by coalesced_mmio_in_range()
rejecting negative lengths (commit 1a214246cbb4).  Factor both checks into
kvm_is_valid_io_zone() and reject with -EINVAL.

Note that this makes KVM_UNREGISTER_COALESCED_MMIO return -EINVAL for a
zone that could never have been registered, instead of the old "0 but the
zone stays" behaviour.  QEMU never registers a zone larger than the
coalesced ring window, so no in-tree user is affected.

Fixes: 743eeb0b01d2 ("KVM: Intelligent device lookup on I/O bus")
Cc: stable@vger.kernel.org
Signed-off-by: Hang Nan <2122295973@qq.com>
---
v2:
 - recipients taken from scripts/get_maintainer.pl (Paolo and the list in To:,
   you and Carlos in Cc:), sorry about that
 - dropped the comment
 - folded the validation into kvm_is_valid_io_zone() and used it from
   kvm_vm_ioctl_unregister_coalesced_mmio() as well

On the unregister side: agreed, it only survives today because
coalesced_mmio_in_range() rejects negative lengths (1a214246cbb4).  Validating
there does change the return value for a zone that can no longer be registered
at all, from 0 to -EINVAL, so I called it out in the changelog -- it is an
ABI-visible difference for anyone who relied on the old behaviour.

Verified before and after the patch on v7.3-11778-g5b0764a53235, same .config,
unprivileged /dev/kvm, same initrd:

  before: REGISTER {addr=0x1000, size=0x80000000}          -> 0
          UNREGISTER {same}                                -> 0 (zone stays)
          998 bad zones accepted, then
          REGISTER {addr=0x900000, size=0x1000}            -> -ENOSPC
  after:  REGISTER {addr=0x1000, size=0x80000000}          -> -EINVAL
          UNREGISTER {same}                                -> -EINVAL
          1200 attempts, 0 accepted
          REGISTER {addr=0x900000, size=0x1000}            -> 0
          UNREGISTER {same}                                -> 0

The reproducer is ~150 lines of static C that only opens /dev/kvm; happy to
post it if it is useful.

Hang


virt/kvm/coalesced_mmio.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
index 3b5430036cae..4afa86b72cf6 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -121,13 +121,24 @@ void kvm_coalesced_mmio_free(struct kvm *kvm)
 		free_page((unsigned long)kvm->coalesced_mmio_ring);
 }
 
+static bool kvm_is_valid_io_zone(struct kvm_coalesced_mmio_zone *zone)
+{
+	if (zone->pio != 1 && zone->pio != 0)
+		return false;
+
+	if (zone->size > INT_MAX)
+		return false;
+
+	return true;
+}
+
 int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
 					 struct kvm_coalesced_mmio_zone *zone)
 {
 	int ret;
 	struct kvm_coalesced_mmio_dev *dev;
 
-	if (zone->pio != 1 && zone->pio != 0)
+	if (!kvm_is_valid_io_zone(zone))
 		return -EINVAL;
 
 	dev = kzalloc_obj(struct kvm_coalesced_mmio_dev, GFP_KERNEL_ACCOUNT);
@@ -162,7 +173,7 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
 	struct kvm_coalesced_mmio_dev *dev, *tmp;
 	int r;
 
-	if (zone->pio != 1 && zone->pio != 0)
+	if (!kvm_is_valid_io_zone(zone))
 		return -EINVAL;
 
 	mutex_lock(&kvm->slots_lock);