[PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[Hongfu Li]

Several mmap() calls lack error checks and would crash on failure.
Add the missing checks. Also replace bare (void *)-1 with the
standard MAP_FAILED macro in protection_keys.c for consistency.

Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
 tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 ++
 tools/testing/selftests/mm/protection_keys.c       | 9 +++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index 302fef54049c..4637809192f9 100644
--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
+++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -317,6 +317,7 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
 	/* Set up alternate signal stack that will use the default MPK */
 	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
 			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	assert(sigstack.ss_sp != MAP_FAILED);
 	sigstack.ss_flags = 0;
 	sigstack.ss_size = STACK_SIZE;
 
@@ -490,6 +491,7 @@ static void test_pkru_sigreturn(void)
 	/* Set up alternate signal stack that will use the default MPK */
 	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
 			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	assert(sigstack.ss_sp != MAP_FAILED);
 	sigstack.ss_flags = 0;
 	sigstack.ss_size = STACK_SIZE;
 
diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index 2085982dba69..d53bdc540a74 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -667,7 +667,7 @@ static void *malloc_pkey_with_mprotect(long size, int prot, u16 pkey)
 			size, prot, pkey);
 	pkey_assert(pkey < NR_PKEYS);
 	ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
-	pkey_assert(ptr != (void *)-1);
+	pkey_assert(ptr != MAP_FAILED);
 	ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
 	pkey_assert(!ret);
 	record_pkey_malloc(ptr, size, prot);
@@ -690,7 +690,7 @@ static void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
 	 */
 	size = ALIGN_UP(size, HPAGE_SIZE * 2);
 	ptr = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
-	pkey_assert(ptr != (void *)-1);
+	pkey_assert(ptr != MAP_FAILED);
 	record_pkey_malloc(ptr, size, prot);
 	mprotect_pkey(ptr, size, prot, pkey);
 
@@ -770,7 +770,7 @@ static void *malloc_pkey_hugetlb(long size, int prot, u16 pkey)
 	size = ALIGN_UP(size, HPAGE_SIZE * 2);
 	pkey_assert(pkey < NR_PKEYS);
 	ptr = mmap(NULL, size, PROT_NONE, flags, -1, 0);
-	pkey_assert(ptr != (void *)-1);
+	pkey_assert(ptr != MAP_FAILED);
 	mprotect_pkey(ptr, size, prot, pkey);
 
 	record_pkey_malloc(ptr, size, prot);
@@ -1217,6 +1217,7 @@ static void arch_force_pkey_reg_init(void)
 	 * doing the XSAVE size enumeration dance.
 	 */
 	buf = mmap(NULL, 1*MB, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+	pkey_assert(buf != MAP_FAILED);
 
 	/* These __builtins require compiling with -mxsave */
 
@@ -1775,7 +1776,7 @@ int main(void)
 		printf("running PKEY tests for unsupported CPU/OS\n");
 
 		ptr  = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
-		assert(ptr != (void *)-1);
+		assert(ptr != MAP_FAILED);
 		test_mprotect_pkey_on_unsupported_cpu(ptr, 1);
 		exit(0);
 	}
-- 
2.25.1

Re: [PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[Lorenzo Stoakes]

Hmm you're sending this separete from the other MAP_FAILED checks, and not
referencing that in any way? (original patch at [0]).

Please just send this as a 2 patch series _with a cover letter_ and both patches
in-reply-to the cover letter.

Also make sure to propagate tags correctly.

[0]:https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/

On Mon, May 18, 2026 at 04:21:20PM +0800, Hongfu Li wrote:
> Several mmap() calls lack error checks and would crash on failure.
> Add the missing checks. Also replace bare (void *)-1 with the

Well you're assert()'ing so you're causing a crash on failure anyway?

I'd just say that you are adding missing checks against the mmap() return value,
as well as improving readability and consistency by replacing (void *)-1 with
MAP_FAILED in instances where that was used rather than MAP_FAILED.

> standard MAP_FAILED macro in protection_keys.c for consistency.
>
> Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>

Some comments below.

> ---
>  tools/testing/selftests/mm/pkey_sighandler_tests.c | 2 ++
>  tools/testing/selftests/mm/protection_keys.c       | 9 +++++----
>  2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> index 302fef54049c..4637809192f9 100644
> --- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
> +++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> @@ -317,6 +317,7 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
>  	/* Set up alternate signal stack that will use the default MPK */
>  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
>  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	assert(sigstack.ss_sp != MAP_FAILED);

Why not pkey_assert()?

>  	sigstack.ss_flags = 0;
>  	sigstack.ss_size = STACK_SIZE;
>
> @@ -490,6 +491,7 @@ static void test_pkru_sigreturn(void)
>  	/* Set up alternate signal stack that will use the default MPK */
>  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
>  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	assert(sigstack.ss_sp != MAP_FAILED);

Why not pkey_assert()?

>  	sigstack.ss_flags = 0;
>  	sigstack.ss_size = STACK_SIZE;
>
> diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
> index 2085982dba69..d53bdc540a74 100644
> --- a/tools/testing/selftests/mm/protection_keys.c
> +++ b/tools/testing/selftests/mm/protection_keys.c
> @@ -667,7 +667,7 @@ static void *malloc_pkey_with_mprotect(long size, int prot, u16 pkey)
>  			size, prot, pkey);
>  	pkey_assert(pkey < NR_PKEYS);
>  	ptr = mmap(NULL, size, prot, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> -	pkey_assert(ptr != (void *)-1);
> +	pkey_assert(ptr != MAP_FAILED);
>  	ret = mprotect_pkey((void *)ptr, PAGE_SIZE, prot, pkey);
>  	pkey_assert(!ret);
>  	record_pkey_malloc(ptr, size, prot);
> @@ -690,7 +690,7 @@ static void *malloc_pkey_anon_huge(long size, int prot, u16 pkey)
>  	 */
>  	size = ALIGN_UP(size, HPAGE_SIZE * 2);
>  	ptr = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> -	pkey_assert(ptr != (void *)-1);
> +	pkey_assert(ptr != MAP_FAILED);
>  	record_pkey_malloc(ptr, size, prot);
>  	mprotect_pkey(ptr, size, prot, pkey);
>
> @@ -770,7 +770,7 @@ static void *malloc_pkey_hugetlb(long size, int prot, u16 pkey)
>  	size = ALIGN_UP(size, HPAGE_SIZE * 2);
>  	pkey_assert(pkey < NR_PKEYS);
>  	ptr = mmap(NULL, size, PROT_NONE, flags, -1, 0);
> -	pkey_assert(ptr != (void *)-1);
> +	pkey_assert(ptr != MAP_FAILED);
>  	mprotect_pkey(ptr, size, prot, pkey);
>
>  	record_pkey_malloc(ptr, size, prot);
> @@ -1217,6 +1217,7 @@ static void arch_force_pkey_reg_init(void)
>  	 * doing the XSAVE size enumeration dance.
>  	 */
>  	buf = mmap(NULL, 1*MB, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> +	pkey_assert(buf != MAP_FAILED);
>
>  	/* These __builtins require compiling with -mxsave */
>
> @@ -1775,7 +1776,7 @@ int main(void)
>  		printf("running PKEY tests for unsupported CPU/OS\n");
>
>  		ptr  = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> -		assert(ptr != (void *)-1);
> +		assert(ptr != MAP_FAILED);

Probably best to convert to pkey_assert() at the same time?

>  		test_mprotect_pkey_on_unsupported_cpu(ptr, 1);
>  		exit(0);
>  	}
> --
> 2.25.1
>

Cheers, Lorenzo

Re: [PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[Hongfu Li]

Hi Lorenzo, 
Thanks for the review comments.

> Hmm you're sending this separete from the other MAP_FAILED checks, and not
> referencing that in any way? (original patch at [0]).
> 
> Please just send this as a 2 patch series _with a cover letter_ and both patches
> in-reply-to the cover letter.
> 
> Also make sure to propagate tags correctly.
> 
> [0]:https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/

The first patch has already been merged into the mm-new branch:
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-new&id=ffe64def0071989cff47b5525d38f5e558c637c3

For this reason, I split this one out separately to avoid confusion.

> On Mon, May 18, 2026 at 04:21:20PM +0800, Hongfu Li wrote:
> > Several mmap() calls lack error checks and would crash on failure.
> > Add the missing checks. Also replace bare (void *)-1 with the
> 
> Well you're assert()'ing so you're causing a crash on failure anyway?
> 
> I'd just say that you are adding missing checks against the mmap() return value,
> as well as improving readability and consistency by replacing (void *)-1 with
> MAP_FAILED in instances where that was used rather than MAP_FAILED.

Thanks for pointing this out, I will correct it in v2.

> > diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > index 302fef54049c..4637809192f9 100644
> > --- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > +++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > @@ -317,6 +317,7 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
> >  	/* Set up alternate signal stack that will use the default MPK */
> >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > +	assert(sigstack.ss_sp != MAP_FAILED);
> 
> Why not pkey_assert()?
> 
> >  	sigstack.ss_flags = 0;
> >  	sigstack.ss_size = STACK_SIZE;
> >
> > @@ -490,6 +491,7 @@ static void test_pkru_sigreturn(void)
> >  	/* Set up alternate signal stack that will use the default MPK */
> >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > +	assert(sigstack.ss_sp != MAP_FAILED);
> 
> Why not pkey_assert()?

protection_keys.c executes numerous tests in loops across multiple iterations, 
so the test_nr and iteration_nr printed by pkey_assert help easily locate the 
exact failed test case and iteration.
In contrast, pkey_sighandler_tests.c consists of only a few standalone test 
functions invoked once each, so plain assert providing file and line information
should suffice to locate failures.

> > @@ -1775,7 +1776,7 @@ int main(void)
> >  		printf("running PKEY tests for unsupported CPU/OS\n");
> >
> >  		ptr  = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> > -		assert(ptr != (void *)-1);
> > +		assert(ptr != MAP_FAILED);
> 
> Probably best to convert to pkey_assert() at the same time?

This is a pre-test initialization path that runs before the test
loop, so test_nr and iteration_nr (used in pkey_assert for diagnostic
output) are not yet set up at this point.  
Would using plain assert() here be more appropriate?

Best regards,
Hongfu

Re: [PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[Lorenzo Stoakes]

On Tue, May 19, 2026 at 05:16:26PM +0800, Hongfu Li wrote:
> Hi Lorenzo,
> Thanks for the review comments.
>
> > Hmm you're sending this separete from the other MAP_FAILED checks, and not
> > referencing that in any way? (original patch at [0]).
> >
> > Please just send this as a 2 patch series _with a cover letter_ and both patches
> > in-reply-to the cover letter.
> >
> > Also make sure to propagate tags correctly.
> >
> > [0]:https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/
>
> The first patch has already been merged into the mm-new branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-new&id=ffe64def0071989cff47b5525d38f5e558c637c3
>
> For this reason, I split this one out separately to avoid confusion.

Hmm ok so you sent a v2 that was rejected [1], you were given feedback for a
respin but the v1 has been taken + not updated?... That's really not how the
process is supposed to work :/

Bit of a mess, Andrew - maybe best to keep the v1 then, and Hongfu - you can
respin this as requested?

[1]: https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/

>
> > On Mon, May 18, 2026 at 04:21:20PM +0800, Hongfu Li wrote:
> > > Several mmap() calls lack error checks and would crash on failure.
> > > Add the missing checks. Also replace bare (void *)-1 with the
> >
> > Well you're assert()'ing so you're causing a crash on failure anyway?
> >
> > I'd just say that you are adding missing checks against the mmap() return value,
> > as well as improving readability and consistency by replacing (void *)-1 with
> > MAP_FAILED in instances where that was used rather than MAP_FAILED.
>
> Thanks for pointing this out, I will correct it in v2.
>
> > > diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > index 302fef54049c..4637809192f9 100644
> > > --- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > +++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > @@ -317,6 +317,7 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
> > >  	/* Set up alternate signal stack that will use the default MPK */
> > >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> > >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > +	assert(sigstack.ss_sp != MAP_FAILED);
> >
> > Why not pkey_assert()?
> >
> > >  	sigstack.ss_flags = 0;
> > >  	sigstack.ss_size = STACK_SIZE;
> > >
> > > @@ -490,6 +491,7 @@ static void test_pkru_sigreturn(void)
> > >  	/* Set up alternate signal stack that will use the default MPK */
> > >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> > >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > +	assert(sigstack.ss_sp != MAP_FAILED);
> >
> > Why not pkey_assert()?
>
> protection_keys.c executes numerous tests in loops across multiple iterations,
> so the test_nr and iteration_nr printed by pkey_assert help easily locate the
> exact failed test case and iteration.
> In contrast, pkey_sighandler_tests.c consists of only a few standalone test
> functions invoked once each, so plain assert providing file and line information
> should suffice to locate failures.

Why would we not want more information here? This argument doesn't hold any
water, please use pkey_assert().

(BTW This reads like an AI generated sentence. We're fine with you using AI to
assist with English for instance, but please make sure it's your own thoughts!)

>
> > > @@ -1775,7 +1776,7 @@ int main(void)
> > >  		printf("running PKEY tests for unsupported CPU/OS\n");
> > >
> > >  		ptr  = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> > > -		assert(ptr != (void *)-1);
> > > +		assert(ptr != MAP_FAILED);
> >
> > Probably best to convert to pkey_assert() at the same time?
>
> This is a pre-test initialization path that runs before the test
> loop, so test_nr and iteration_nr (used in pkey_assert for diagnostic
> output) are not yet set up at this point.
> Would using plain assert() here be more appropriate?

OK that's gross, please just replace it with a test failure kmsg_xxx() whatever
it is, and a return EXIT_FAILURE; or something since you're in main().

>
> Best regards,
> Hongfu

Cheers, Lorenzo

Re: [PATCH] selftests/mm: add missing mmap() return checks in pkey tests

[Hongfu Li]

Hi Lorenzo,
Thanks for the review comments.

> > > Hmm you're sending this separete from the other MAP_FAILED checks, and not
> > > referencing that in any way? (original patch at [0]).
> > >
> > > Please just send this as a 2 patch series _with a cover letter_ and both patches
> > > in-reply-to the cover letter.
> > >
> > > Also make sure to propagate tags correctly.
> > >
> > > [0]:https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/
> >
> > The first patch has already been merged into the mm-new branch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-new&id=ffe64def0071989cff47b5525d38f5e558c637c3
> >
> > For this reason, I split this one out separately to avoid confusion.
> 
> Hmm ok so you sent a v2 that was rejected [1], you were given feedback for a
> respin but the v1 has been taken + not updated?... That's really not how the
> process is supposed to work :/
> 
> Bit of a mess, Andrew - maybe best to keep the v1 then, and Hongfu - you can
> respin this as requested?
> 
> [1]: https://lore.kernel.org/all/20260513095609.789935-1-lihongfu@kylinos.cn/

Sorry for the messy patch arrangement before, I'll respin this patch strictly
following your requirements.

> >
> > > On Mon, May 18, 2026 at 04:21:20PM +0800, Hongfu Li wrote:
> > > > Several mmap() calls lack error checks and would crash on failure.
> > > > Add the missing checks. Also replace bare (void *)-1 with the
> > >
> > > Well you're assert()'ing so you're causing a crash on failure anyway?
> > >
> > > I'd just say that you are adding missing checks against the mmap() return value,
> > > as well as improving readability and consistency by replacing (void *)-1 with
> > > MAP_FAILED in instances where that was used rather than MAP_FAILED.
> >
> > Thanks for pointing this out, I will correct it in v2.
> >
> > > > diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > > index 302fef54049c..4637809192f9 100644
> > > > --- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > > +++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
> > > > @@ -317,6 +317,7 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
> > > >  	/* Set up alternate signal stack that will use the default MPK */
> > > >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> > > >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > +	assert(sigstack.ss_sp != MAP_FAILED);
> > >
> > > Why not pkey_assert()?
> > >
> > > >  	sigstack.ss_flags = 0;
> > > >  	sigstack.ss_size = STACK_SIZE;
> > > >
> > > > @@ -490,6 +491,7 @@ static void test_pkru_sigreturn(void)
> > > >  	/* Set up alternate signal stack that will use the default MPK */
> > > >  	sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
> > > >  			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> > > > +	assert(sigstack.ss_sp != MAP_FAILED);
> > >
> > > Why not pkey_assert()?
> >
> > protection_keys.c executes numerous tests in loops across multiple iterations,
> > so the test_nr and iteration_nr printed by pkey_assert help easily locate the
> > exact failed test case and iteration.
> > In contrast, pkey_sighandler_tests.c consists of only a few standalone test
> > functions invoked once each, so plain assert providing file and line information
> > should suffice to locate failures.
> 
> Why would we not want more information here? This argument doesn't hold any
> water, please use pkey_assert().

Sure, I will use pkey_assert() and adjust it in the next revision.

> > (BTW This reads like an AI generated sentence. We're fine with you using AI to
> > assist with English for instance, but please make sure it's your own thoughts!)
 
 Sorry for that! I only use AI to polish my English expressions, and all the ideas
 are totally my own.
 
> >
> > > > @@ -1775,7 +1776,7 @@ int main(void)
> > > >  		printf("running PKEY tests for unsupported CPU/OS\n");
> > > >
> > > >  		ptr  = mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
> > > > -		assert(ptr != (void *)-1);
> > > > +		assert(ptr != MAP_FAILED);
> > >
> > > Probably best to convert to pkey_assert() at the same time?
> >
> > This is a pre-test initialization path that runs before the test
> > loop, so test_nr and iteration_nr (used in pkey_assert for diagnostic
> > output) are not yet set up at this point.
> > Would using plain assert() here be more appropriate?
> 
> OK that's gross, please just replace it with a test failure kmsg_xxx() whatever
> it is, and a return EXIT_FAILURE; or something since you're in main().

Got it. I will remove the assert here completely, add standard error reporting
and return EXIT_FAILURE directly on failure.

Thanks again for your patient guidance and rigorous review!

Best regards,
Hongfu