Re: [PATCH v5.5 02/30] KVM: Disallow user memslot with size that exceeds "unsigned long"

"Maciej S. Szmigiero" <[email protected]>
Newsgroups org.kernel.vger.kvm-ppc,dev.linux.lists.kvmarm,org.infradead.lists.kvm-riscv,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-riscv,org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mips
Message-ID <[email protected]>
On 04.11.2021 01:25, Sean Christopherson wrote:
> Reject userspace memslots whose size exceeds the storage capacity of an
> "unsigned long".  KVM's uAPI takes the size as u64 to support large slots
> on 64-bit hosts, but does not account for the size being truncated on
> 32-bit hosts in various flows.  The access_ok() check on the userspace
> virtual address in particular casts the size to "unsigned long" and will
> check the wrong number of bytes.
> 
> KVM doesn't actually support slots whose size doesn't fit in an "unsigned
> long", e.g. KVM's internal kvm_memory_slot.npages is an "unsigned long",
> not a "u64", and misc arch specific code follows that behavior.
> 
> Fixes: fa3d315a4ce2 ("KVM: Validate userspace_addr of memslot when registered")
> Cc: [email protected]
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
>   virt/kvm/kvm_main.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 99e69375c4c9..83287730389f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1689,7 +1689,8 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   	id = (u16)mem->slot;
>   
>   	/* General sanity checks */
> -	if (mem->memory_size & (PAGE_SIZE - 1))
> +	if ((mem->memory_size & (PAGE_SIZE - 1)) ||
> +	    (mem->memory_size != (unsigned long)mem->memory_size))
>   		return -EINVAL;
>   	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
>   		return -EINVAL;
> 

Reviewed-by: Maciej S. Szmigiero <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.