Re: [PATCH 1/2] KVM: arm64: vgic: check redist region is not above the VM IPA size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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))




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux