KVM: arm64: vgic: Don't truncate offsets in vgic_find_mmio_region()
PATCH v2 1/1 KVM: arm64: vgic: Don't truncate offsets in vgic_find_mmio_region()
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.
---
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(-)
arch/arm64/kvm/vgic/vgic-mmio.c +2 -2
@@ -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);
arch/arm64/kvm/vgic/vgic-mmio.h +1 -1
@@ -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
Signature · 1 line
2.53.0