Re: [PATCH] selftests/mm: Fix incorrect mmap() error handling with NULL instead of MAP_FAILED

[Dev Jain]

On 13/05/26 8:22 am, Hongfu Li wrote:
> mmap() returns MAP_FAILED, which is defined as (void *)-1, on error,
> not NULL.  Several selftests incorrectly check the return value of
> mmap() using !ptr or ptr == NULL, which would erroneously treat
> MAP_FAILED as a valid pointer since MAP_FAILED is non-zero and
> non-NULL.  This can lead to segfaults when mmap() actually fails
> under memory pressure.
> 
> Signed-off-by: Hongfu Li <lihon...@kylinos.cn>

Hopefully you have fixed all instances in selftests/mm :)

Reviewed-by: Dev Jain <dev.j...@arm.com>

> ---
>  tools/testing/selftests/mm/ksm_tests.c     | 2 +-
>  tools/testing/selftests/mm/madv_populate.c | 2 +-
>  tools/testing/selftests/mm/soft-dirty.c    | 4 ++--
>  tools/testing/selftests/mm/vm_util.c       | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/mm/ksm_tests.c 
> b/tools/testing/selftests/mm/ksm_tests.c
> index a0b48b839d54..ed481f817282 100644
> --- a/tools/testing/selftests/mm/ksm_tests.c
> +++ b/tools/testing/selftests/mm/ksm_tests.c
> @@ -174,7 +174,7 @@ static void  *allocate_memory(void *ptr, int prot, int 
> mapping, char data, size_
>  {
>       void *map_ptr = mmap(ptr, map_size, PROT_WRITE, mapping, -1, 0);
>  
> -     if (!map_ptr) {
> +     if (map_ptr == MAP_FAILED) {
>               perror("mmap");
>               return NULL;
>       }
> diff --git a/tools/testing/selftests/mm/madv_populate.c 
> b/tools/testing/selftests/mm/madv_populate.c
> index 88050e0f829a..7fce5d0b622b 100644
> --- a/tools/testing/selftests/mm/madv_populate.c
> +++ b/tools/testing/selftests/mm/madv_populate.c
> @@ -34,7 +34,7 @@ static void sense_support(void)
>  
>       addr = mmap(0, pagesize, PROT_READ | PROT_WRITE,
>                   MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> -     if (!addr)
> +     if (addr == MAP_FAILED)
>               ksft_exit_fail_msg("mmap failed\n");
>  
>       ret = madvise(addr, pagesize, MADV_POPULATE_READ);
> diff --git a/tools/testing/selftests/mm/soft-dirty.c 
> b/tools/testing/selftests/mm/soft-dirty.c
> index bcfcac99b436..67c26c265880 100644
> --- a/tools/testing/selftests/mm/soft-dirty.c
> +++ b/tools/testing/selftests/mm/soft-dirty.c
> @@ -143,7 +143,7 @@ static void test_mprotect(int pagemap_fd, int pagesize, 
> bool anon)
>       if (anon) {
>               map = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
>                          MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> -             if (!map)
> +             if (map == MAP_FAILED)
>                       ksft_exit_fail_msg("anon mmap failed\n");
>       } else {
>               test_fd = open(fname, O_RDWR | O_CREAT, 0664);
> @@ -155,7 +155,7 @@ static void test_mprotect(int pagemap_fd, int pagesize, 
> bool anon)
>               ftruncate(test_fd, pagesize);
>               map = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
>                          MAP_SHARED, test_fd, 0);
> -             if (!map)
> +             if (map == MAP_FAILED)
>                       ksft_exit_fail_msg("file mmap failed\n");
>       }
>  
> diff --git a/tools/testing/selftests/mm/vm_util.c 
> b/tools/testing/selftests/mm/vm_util.c
> index db94564f4431..63aaa2d9ec0b 100644
> --- a/tools/testing/selftests/mm/vm_util.c
> +++ b/tools/testing/selftests/mm/vm_util.c
> @@ -463,7 +463,7 @@ bool softdirty_supported(void)
>       /* New mappings are expected to be marked with VM_SOFTDIRTY (sd). */
>       addr = mmap(0, pagesize, PROT_READ | PROT_WRITE,
>                   MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> -     if (!addr)
> +     if (addr == MAP_FAILED)
>               ksft_exit_fail_msg("mmap failed\n");
>  
>       supported = check_vmflag(addr, "sd");