On Mon, Sep 20, 2021 at 01:30:40PM +0100, Marc Zyngier wrote: > Hi Ricardo, > > On Fri, 10 Sep 2021 01:49:18 +0100, > Ricardo Koller <ricarkol@xxxxxxxxxx> wrote: > > > > Verify that the redistributor regions do not extend beyond the > > VM-specified IPA size (phys_size). This can happen when using > > KVM_VGIC_V3_ADDR_TYPE_REDIST or KVM_VGIC_V3_ADDR_TYPE_REDIST_REGIONS > > with: > > > > base + size > phys_size AND base < phys_size > > > > Add the missing check into vgic_v3_alloc_redist_region() which is called > > when setting the regions, and into vgic_v3_check_base() which is called > > when attempting the first vcpu-run. The vcpu-run check does not apply to > > KVM_VGIC_V3_ADDR_TYPE_REDIST_REGIONS because the regions size is known > > before the first vcpu-run. Finally, this patch also enables some extra > > tests in vgic_v3_alloc_redist_region() by calculating "size" early for > > the legacy redist api. > > > > Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx> > > --- > > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 7 ++++++- > > arch/arm64/kvm/vgic/vgic-v3.c | 4 ++++ > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > > index a09cdc0b953c..055671bede85 100644 > > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c > > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > > @@ -796,7 +796,9 @@ static int vgic_v3_alloc_redist_region(struct kvm *kvm, uint32_t index, > > struct vgic_dist *d = &kvm->arch.vgic; > > struct vgic_redist_region *rdreg; > > struct list_head *rd_regions = &d->rd_regions; > > - size_t size = count * KVM_VGIC_V3_REDIST_SIZE; > > + int nr_vcpus = atomic_read(&kvm->online_vcpus); > > + size_t size = count ? count * KVM_VGIC_V3_REDIST_SIZE : > > + nr_vcpus * KVM_VGIC_V3_REDIST_SIZE; > > int ret; > > > > /* cross the end of memory ? */ > > @@ -834,6 +836,9 @@ static int vgic_v3_alloc_redist_region(struct kvm *kvm, uint32_t index, > > if (vgic_v3_rdist_overlap(kvm, base, size)) > > return -EINVAL; > > > > + if (base + size > kvm_phys_size(kvm)) > > + return -E2BIG; > > + > > rdreg = kzalloc(sizeof(*rdreg), GFP_KERNEL); > > if (!rdreg) > > return -ENOMEM; > > 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; > > } > > > > if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base)) > > How about vgic-v2? From what I can see, the placement of the > distributor and CPU interface should be subjected to the same checks > (see vgic_v2_check_base()). > Yes, gic-v2 has the same problem. Alexandru first mentioned it here: https://lore.kernel.org/kvmarm/5eb41efd-2ff2-d25b-5801-f4a56457a09f@xxxxxxx/ I'm now working on the next version of the patch which applies to gic-v2 as well. Thanks, Ricardo > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
