Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] KVM: selftests: Fix mapping length truncation in
 m{,un}map()
To:     Zenghui Yu <yuzenghui@huawei.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, vkuznets@redhat.com
Cc:     wanghaibin.wang@huawei.com
References: <20210624070931.565-1-yuzenghui@huawei.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <836e058c-6bb1-7643-98c2-713b111c9fb8@redhat.com>
Date:   Thu, 24 Jun 2021 09:19:09 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210624070931.565-1-yuzenghui@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 24/06/21 09:09, Zenghui Yu wrote:
> max_mem_slots is now declared as uint32_t. The result of (0x200000 * 32767)
> is unexpectedly truncated to be 0xffe00000, whilst we actually need to
> allocate about, 63GB. Cast max_mem_slots to size_t in both mmap() and
> munmap() to fix the length truncation.
> 
> We'll otherwise see the failure on arm64 thanks to the access_ok() checking
> in __kvm_set_memory_region(), as the unmapped VA happen to go beyond the
> task's allowed address space.
> 
>   # ./set_memory_region_test
> Allowed number of memory slots: 32767
> Adding slots 0..32766, each memory region with 2048K size
> ==== Test Assertion Failure ====
>    set_memory_region_test.c:391: ret == 0
>    pid=94861 tid=94861 errno=22 - Invalid argument
>       1	0x00000000004015a7: test_add_max_memory_regions at set_memory_region_test.c:389
>       2	 (inlined by) main at set_memory_region_test.c:426
>       3	0x0000ffffb8e67bdf: ?? ??:0
>       4	0x00000000004016db: _start at :?
>    KVM_SET_USER_MEMORY_REGION IOCTL failed,
>    rc: -1 errno: 22 slot: 2615
> 
> Fixes: 3bf0fcd75434 ("KVM: selftests: Speed up set_memory_region_test")
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>   tools/testing/selftests/kvm/set_memory_region_test.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
> index d79d58eada9f..85b18bb8f762 100644
> --- a/tools/testing/selftests/kvm/set_memory_region_test.c
> +++ b/tools/testing/selftests/kvm/set_memory_region_test.c
> @@ -376,7 +376,7 @@ static void test_add_max_memory_regions(void)
>   	pr_info("Adding slots 0..%i, each memory region with %dK size\n",
>   		(max_mem_slots - 1), MEM_REGION_SIZE >> 10);
>   
> -	mem = mmap(NULL, MEM_REGION_SIZE * max_mem_slots + alignment,
> +	mem = mmap(NULL, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment,
>   		   PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>   	TEST_ASSERT(mem != MAP_FAILED, "Failed to mmap() host");
>   	mem_aligned = (void *)(((size_t) mem + alignment - 1) & ~(alignment - 1));
> @@ -401,7 +401,7 @@ static void test_add_max_memory_regions(void)
>   	TEST_ASSERT(ret == -1 && errno == EINVAL,
>   		    "Adding one more memory slot should fail with EINVAL");
>   
> -	munmap(mem, MEM_REGION_SIZE * max_mem_slots + alignment);
> +	munmap(mem, (size_t)max_mem_slots * MEM_REGION_SIZE + alignment);
>   	munmap(mem_extra, MEM_REGION_SIZE);
>   	kvm_vm_free(vm);
>   }
> 

Queued, thanks.

Paolo