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 <[email protected]>
> ---
> 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)
> --
> 2.51.2
>
--
Sincerely yours,
Mike.
