Commit 112b0b8f authored by Andre Przywara's avatar Andre Przywara Committed by Marc Zyngier

KVM: arm/arm64: vgic: Prevent access to invalid SPIs

In our VGIC implementation we limit the number of SPIs to a number
that the userland application told us. Accordingly we limit the
allocation of memory for virtual IRQs to that number.
However in our MMIO dispatcher we didn't check if we ever access an
IRQ beyond that limit, leading to out-of-bound accesses.
Add a test against the number of allocated SPIs in check_region().
Adjust the VGIC_ADDR_TO_INT macro to avoid an actual division, which
is not implemented on ARM(32).

[maz: cleaned-up original patch]

Cc: stable@vger.kernel.org
Reviewed-by: default avatarChristoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: default avatarAndre Przywara <andre.przywara@arm.com>
Signed-off-by: default avatarMarc Zyngier <marc.zyngier@arm.com>
parent 94d0e598
...@@ -453,17 +453,33 @@ struct vgic_io_device *kvm_to_vgic_iodev(const struct kvm_io_device *dev) ...@@ -453,17 +453,33 @@ struct vgic_io_device *kvm_to_vgic_iodev(const struct kvm_io_device *dev)
return container_of(dev, struct vgic_io_device, dev); return container_of(dev, struct vgic_io_device, dev);
} }
static bool check_region(const struct vgic_register_region *region, static bool check_region(const struct kvm *kvm,
const struct vgic_register_region *region,
gpa_t addr, int len) gpa_t addr, int len)
{ {
if ((region->access_flags & VGIC_ACCESS_8bit) && len == 1) int flags, nr_irqs = kvm->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS;
return true;
if ((region->access_flags & VGIC_ACCESS_32bit) && switch (len) {
len == sizeof(u32) && !(addr & 3)) case sizeof(u8):
return true; flags = VGIC_ACCESS_8bit;
if ((region->access_flags & VGIC_ACCESS_64bit) && break;
len == sizeof(u64) && !(addr & 7)) case sizeof(u32):
return true; flags = VGIC_ACCESS_32bit;
break;
case sizeof(u64):
flags = VGIC_ACCESS_64bit;
break;
default:
return false;
}
if ((region->access_flags & flags) && IS_ALIGNED(addr, len)) {
if (!region->bits_per_irq)
return true;
/* Do we access a non-allocated IRQ? */
return VGIC_ADDR_TO_INTID(addr, region->bits_per_irq) < nr_irqs;
}
return false; return false;
} }
...@@ -477,7 +493,7 @@ static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, ...@@ -477,7 +493,7 @@ static int dispatch_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions, region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
addr - iodev->base_addr); addr - iodev->base_addr);
if (!region || !check_region(region, addr, len)) { if (!region || !check_region(vcpu->kvm, region, addr, len)) {
memset(val, 0, len); memset(val, 0, len);
return 0; return 0;
} }
...@@ -510,10 +526,7 @@ static int dispatch_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev, ...@@ -510,10 +526,7 @@ static int dispatch_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions, region = vgic_find_mmio_region(iodev->regions, iodev->nr_regions,
addr - iodev->base_addr); addr - iodev->base_addr);
if (!region) if (!region || !check_region(vcpu->kvm, region, addr, len))
return 0;
if (!check_region(region, addr, len))
return 0; return 0;
switch (iodev->iodev_type) { switch (iodev->iodev_type) {
......
...@@ -50,15 +50,15 @@ extern struct kvm_io_device_ops kvm_io_gic_ops; ...@@ -50,15 +50,15 @@ extern struct kvm_io_device_ops kvm_io_gic_ops;
#define VGIC_ADDR_IRQ_MASK(bits) (((bits) * 1024 / 8) - 1) #define VGIC_ADDR_IRQ_MASK(bits) (((bits) * 1024 / 8) - 1)
/* /*
* (addr & mask) gives us the byte offset for the INT ID, so we want to * (addr & mask) gives us the _byte_ offset for the INT ID.
* divide this with 'bytes per irq' to get the INT ID, which is given * We multiply this by 8 the get the _bit_ offset, then divide this by
* by '(bits) / 8'. But we do this with fixed-point-arithmetic and * the number of bits to learn the actual INT ID.
* take advantage of the fact that division by a fraction equals * But instead of a division (which requires a "long long div" implementation),
* multiplication with the inverted fraction, and scale up both the * we shift by the binary logarithm of <bits>.
* numerator and denominator with 8 to support at most 64 bits per IRQ: * This assumes that <bits> is a power of two.
*/ */
#define VGIC_ADDR_TO_INTID(addr, bits) (((addr) & VGIC_ADDR_IRQ_MASK(bits)) * \ #define VGIC_ADDR_TO_INTID(addr, bits) (((addr) & VGIC_ADDR_IRQ_MASK(bits)) * \
64 / (bits) / 8) 8 >> ilog2(bits))
/* /*
* Some VGIC registers store per-IRQ information, with a different number * Some VGIC registers store per-IRQ information, with a different number
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment