[PATCH v2] KVM: arm64: vgic: Don't truncate offsets in vgic_find_mmio_region()

[PATCH v2] KVM: arm64: vgic: Don't truncate offsets in vgic_find_mmio_region()

[Dan Carpenter]

In vgic_its_has_attr_regs() and vgic_its_attr_regs_access() we take a
u64 "offset" from the user and pass it to vgic_find_mmio_region() to
find the correct region.  The problem is that vgic_find_mmio_region()
truncates the "offset" to 32 bits so we might find a region even
though really we shouldn't.

Later the untruncated offset is used to calculate "addr".

	addr = its->vgic_its_base + offset;

For example, an offset of 0x100000084 finds the 64-bit GITS_CBASER
register at 0x80, and the write reaches update_64bit_reg() with an
offset of 4 into the register and a length of 8.  It exhibits as a
UBSAN warning.

[  183.796046] UBSAN: shift-out-of-bounds in arch/arm64/kvm/vgic/vgic-mmio-v3.c:35:10
[  183.796198] shift exponent 18446744073709551584 is too large for 64-bit type 'long long unsigned int'

The GICv5 IRS code has the same problem in vgic_v5_has_attr_regs() and
vgic_v5_irs_attr_regs_access().

Make vgic_find_mmio_region() and match_region() take the offset as a
u64, so an offset of 2^32 or more no longer matches any region.  The
other callers only pass offsets which fit in 32 bits so they are not
affected.

Fortunately the effect of this bug is very minor.  The worst case is that
the VMM accesses an emulated register and sets something nonsensical
and users are already allowed to do that without the bug.

Fixes: 8331c23c28e8 ("KVM: arm64: vgic-its: Implement vgic_its_has_attr_regs and attr_regs_access")
Fixes: 71091136d145 ("KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region")
Signed-off-by: Dan Carpenter <dan@cryspen.com>
---
v2: Change vgic_find_mmio_region() to take a u64 instead of rejecting
    large offsets in the ITS callers, as Marc suggested.  This also
    fixes the GICv5 IRS code which my previous patch didn't.  Drop the
    Cc: stable.

 arch/arm64/kvm/vgic/vgic-mmio.c | 4 ++--
 arch/arm64/kvm/vgic/vgic-mmio.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c
index fddb9da0403d..59bdec495cd7 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio.c
@@ -820,7 +820,7 @@ void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
 
 static int match_region(const void *key, const void *elt)
 {
-	const unsigned int offset = (unsigned long)key;
+	const u64 offset = (unsigned long)key;
 	const struct vgic_register_region *region = elt;
 
 	if (offset < region->reg_offset)
@@ -834,7 +834,7 @@ static int match_region(const void *key, const void *elt)
 
 const struct vgic_register_region *
 vgic_find_mmio_region(const struct vgic_register_region *regions,
-		      int nr_regions, unsigned int offset)
+		      int nr_regions, u64 offset)
 {
 	return bsearch((void *)(uintptr_t)offset, regions, nr_regions,
 		       sizeof(regions[0]), match_region);
diff --git a/arch/arm64/kvm/vgic/vgic-mmio.h b/arch/arm64/kvm/vgic/vgic-mmio.h
index 38ed730d68ac..6c3c4de6883e 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio.h
+++ b/arch/arm64/kvm/vgic/vgic-mmio.h
@@ -228,6 +228,6 @@ u64 vgic_sanitise_field(u64 reg, u64 field_mask, int field_shift,
 /* Find the proper register handler entry given a certain address offset */
 const struct vgic_register_region *
 vgic_find_mmio_region(const struct vgic_register_region *regions,
-		      int nr_regions, unsigned int offset);
+		      int nr_regions, u64 offset);
 
 #endif
-- 
2.53.0