Hi, On 9/9/21 12:20 PM, Alexandru Elisei wrote: > Hi Ricardo, > > On 9/8/21 10:03 PM, Ricardo Koller wrote: >> Extend vgic_v3_check_base() to verify that the redistributor regions >> don't go above the VM-specified IPA size (phys_size). This can happen >> when using the legacy KVM_VGIC_V3_ADDR_TYPE_REDIST attribute with: >> >> base + size > phys_size AND base < phys_size >> >> vgic_v3_check_base() is used to check the redist regions bases when >> setting them (with the vcpus added so far) and when attempting the first >> vcpu-run. >> >> Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx> >> --- >> arch/arm64/kvm/vgic/vgic-v3.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c >> index 66004f61cd83..5afd9f6f68f6 100644 >> --- a/arch/arm64/kvm/vgic/vgic-v3.c >> +++ b/arch/arm64/kvm/vgic/vgic-v3.c >> @@ -512,6 +512,10 @@ bool vgic_v3_check_base(struct kvm *kvm) >> if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) < >> rdreg->base) >> return false; >> + >> + if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) > >> + kvm_phys_size(kvm)) >> + return false; > Looks to me like this same check (and the overflow one before it) is done when > adding a new Redistributor region in kvm_vgic_addr() -> vgic_v3_set_redist_base() > -> vgic_v3_alloc_redist_region() -> vgic_check_ioaddr(). As far as I can tell, > kvm_vgic_addr() handles both ways of setting the Redistributor address. To me vgic_check_ioaddr() does check the base addr but not the end addr. So looks this fix is needed. As I commented on the selftest patch, I think you should double check your fix also handles the KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION case. In vgic_v3_alloc_redist_region(), in this later case, we know the number of redistributors in the region (count), so it would be easy to check the end addr. But I think this would be a duplicate of your new check as vgic_v3_check_base() also gets called in vgic_register_redist_iodev(). But better to check it ;-) Thanks Eric > > Without this patch, did you manage to set a base address such that base + size > > kvm_phys_size()? > > Thanks, > > Alex > >> } >> >> if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base))
