Re: [PATCH] selftests/mm/uffd: remove static address usage in shmem_allocate_area()

[Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>]

On 11/13/25 12:39 PM, Mike Rapoport wrote:
> On Tue, Nov 11, 2025 at 09:54:27PM +0100, Mehdi Ben Hadj Khelifa wrote:
>> The current shmem_allocate_area() implementation uses a hardcoded virtual
>> base address(BASE_PMD_ADDR) as a hint for mmap() when creating shmem-backed
>> test areas. This approach is fragile and may fail on systems with ASLR or
>> different virtual memory layouts, where the chosen address is unavailable.
>>
>> Replace the static base address with a dynamically reserved address range
>> obtained via mmap(NULL, ..., PROT_NONE). The memfd-backed areas and their
>> alias are then mapped into that reserved region using MAP_FIXED, preserving
>> the original layout and aliasing semantics while avoiding collisions with
>> unrelated mappings.
>>
>> This change improves robustness and portability of the test suite without
>> altering its behavior or coverage.
>>
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>> ---
>> Testing:
>> A diff between running the mm selftests on 6.18-rc5 from before and after
>> the change show no regression on x86_64 architecture with 32GB DDR5 RAM.
>>   tools/testing/selftests/mm/uffd-common.c | 25 +++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mm/uffd-common.c b/tools/testing/selftests/mm/uffd-common.c
>> index 994fe8c03923..492b21c960bb 100644
>> --- a/tools/testing/selftests/mm/uffd-common.c
>> +++ b/tools/testing/selftests/mm/uffd-common.c
>> @@ -6,11 +6,11 @@
>>    */
>>   
>>   #include "uffd-common.h"
>> +#include "asm-generic/mman-common.h"
> 
> Please drop this.
> There's already include <sys/mman.h> via uffd-common.h/vm_util.h.
> 
>>   
>>   uffd_test_ops_t *uffd_test_ops;
>>   uffd_test_case_ops_t *uffd_test_case_ops;
>>   
>> -#define BASE_PMD_ADDR ((void *)(1UL << 30))
>>   
>>   /* pthread_mutex_t starts at page offset 0 */
>>   pthread_mutex_t *area_mutex(char *area, unsigned long nr, uffd_global_test_opts_t *gopts)
>> @@ -142,30 +142,37 @@ static int shmem_allocate_area(uffd_global_test_opts_t *gopts, void **alloc_area
>>   	unsigned long offset = is_src ? 0 : bytes;
>>   	char *p = NULL, *p_alias = NULL;
>>   	int mem_fd = uffd_mem_fd_create(bytes * 2, false);
>> +	size_t region_size = bytes * 2 + hpage_size;
>>   
>> -	/* TODO: clean this up.  Use a static addr is ugly */
>> -	p = BASE_PMD_ADDR;
>> -	if (!is_src)
>> -		/* src map + alias + interleaved hpages */
>> -		p += 2 * (bytes + hpage_size);
>> +	void *reserve = mmap(NULL, region_size, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
>> +			-1, 0);
>> +	if (reserve == MAP_FAILED) {
>> +		close(mem_fd);
>> +		return -errno;
>> +	}
>> +
>> +	p = (char *)reserve;
> 
> No need for casting here.
> 
>>   	p_alias = p;
>>   	p_alias += bytes;
>>   	p_alias += hpage_size;  /* Prevent src/dst VMA merge */
>>   
>> -	*alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
>> +	*alloc_area = mmap(p, bytes, PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED,
>>   			   mem_fd, offset);
>>   	if (*alloc_area == MAP_FAILED) {
>> +		munmap(reserve, region_size);
> 
> I think it'll be more readable to put munmap() after setting *alloc_area to
> NULL.
> 
>>   		*alloc_area = NULL;
>> +		close(mem_fd);
>>   		return -errno;
>>   	}
>>   	if (*alloc_area != p)
>>   		err("mmap of memfd failed at %p", p);
>>   
>> -	area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_SHARED,
>> +	area_alias = mmap(p_alias, bytes, PROT_READ | PROT_WRITE, MAP_FIXED | MAP_SHARED,
>>   			  mem_fd, offset);
>>   	if (area_alias == MAP_FAILED) {
>> -		munmap(*alloc_area, bytes);
>> +		munmap(reserve, region_size);
> 
> Here as well.
> 
>>   		*alloc_area = NULL;
>> +		close(mem_fd);
>>   		return -errno;
>>   	}
>>   	if (area_alias != p_alias)
Thank you for your suggestions, A v2 has already been sent out with your 
suggestions.
Best Regards,
Mehdi Ben Hadj Khelifa>> --
>> 2.51.2
>>
>