* [PATCH v3 1/2] selftests/mm: refactor pkey test helpers and unify assertions
2026-05-27 1:38 [PATCH v3 0/2] selftests/mm: refactor pkey helpers and fix mmap error handling Hongfu Li
@ 2026-05-27 1:38 ` Hongfu Li
2026-05-27 11:32 ` Mike Rapoport
2026-05-27 1:38 ` [PATCH v3 2/2] selftests/mm: add missing mmap() return checks in pkey tests Hongfu Li
1 sibling, 1 reply; 5+ messages in thread
From: Hongfu Li @ 2026-05-27 1:38 UTC (permalink / raw)
To: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, shuah
Cc: linux-mm, linux-kselftest, linux-kernel, Hongfu Li
Move shared tracing and assertion helpers to a common file, then
convert the sighandler test to use pkey_assert() for consistent
diagnostic output, and add per-test tracing around test execution.
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
tools/testing/selftests/mm/pkey-helpers.h | 5 +-
.../selftests/mm/pkey_sighandler_tests.c | 108 +++++++++---------
tools/testing/selftests/mm/pkey_util.c | 81 +++++++++++++
tools/testing/selftests/mm/protection_keys.c | 82 -------------
4 files changed, 137 insertions(+), 139 deletions(-)
diff --git a/tools/testing/selftests/mm/pkey-helpers.h b/tools/testing/selftests/mm/pkey-helpers.h
index 7c29f075e40b..de9052386343 100644
--- a/tools/testing/selftests/mm/pkey-helpers.h
+++ b/tools/testing/selftests/mm/pkey-helpers.h
@@ -68,7 +68,10 @@ static inline void sigsafe_printf(const char *format, ...)
#define dprintf3(args...) dprintf_level(3, args)
#define dprintf4(args...) dprintf_level(4, args)
-extern void abort_hooks(void);
+void cat_into_file(char *str, char *file);
+void tracing_on(void);
+void tracing_off(void);
+void abort_hooks(void);
#define pkey_assert(condition) do { \
if (!(condition)) { \
dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \
diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index 302fef54049c..231dfb079075 100644
--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
+++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -19,7 +19,6 @@
#include <stdint.h>
#include <stdbool.h>
#include <signal.h>
-#include <assert.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <sys/types.h>
@@ -36,6 +35,10 @@ static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
static siginfo_t siginfo = {0};
+int iteration_nr = 1;
+int test_nr;
+int dprint_in_signal;
+
/*
* We need to use inline assembly instead of glibc's syscall because glibc's
* syscall will attempt to access the PLT in order to call a library function
@@ -207,15 +210,14 @@ static void test_sigsegv_handler_with_pkey0_disabled(void)
struct sigaction sa;
pthread_attr_t attr;
pthread_t thr;
+ int ret;
sa.sa_flags = SA_SIGINFO;
sa.sa_sigaction = sigsegv_handler;
sigemptyset(&sa.sa_mask);
- if (sigaction(SIGSEGV, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGSEGV, &sa, NULL);
+ pkey_assert(ret == 0);
memset(&siginfo, 0, sizeof(siginfo));
@@ -247,15 +249,14 @@ static void test_sigsegv_handler_cannot_access_stack(void)
struct sigaction sa;
pthread_attr_t attr;
pthread_t thr;
+ int ret;
sa.sa_flags = SA_SIGINFO;
sa.sa_sigaction = sigsegv_handler;
sigemptyset(&sa.sa_mask);
- if (sigaction(SIGSEGV, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGSEGV, &sa, NULL);
+ pkey_assert(ret == 0);
memset(&siginfo, 0, sizeof(siginfo));
@@ -288,21 +289,20 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
int parent_pid = 0;
int child_pid = 0;
u64 pkey_reg;
+ int ret;
sa.sa_flags = SA_SIGINFO | SA_ONSTACK;
sa.sa_sigaction = sigsegv_handler;
sigemptyset(&sa.sa_mask);
- if (sigaction(SIGSEGV, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGSEGV, &sa, NULL);
+ pkey_assert(ret == 0);
stack = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- assert(stack != MAP_FAILED);
+ pkey_assert(stack != MAP_FAILED);
/* Allow access to MPK 0 and MPK 1 */
pkey_reg = pkey_reg_restrictive_default();
@@ -323,18 +323,18 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
memset(&siginfo, 0, sizeof(siginfo));
/* Use clone to avoid newer glibcs using rseq on new threads */
- long ret = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
- CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
- CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
- CLONE_DETACHED,
- stack + STACK_SIZE,
- &parent_pid,
- &child_pid);
-
- if (ret < 0) {
- errno = -ret;
- perror("clone");
- } else if (ret == 0) {
+ child_pid = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
+ CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
+ CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
+ CLONE_DETACHED,
+ stack + STACK_SIZE,
+ &parent_pid,
+ &child_pid);
+
+ if (child_pid < 0) {
+ errno = -child_pid;
+ pkey_assert(0);
+ } else if (child_pid == 0) {
thread_segv_maperr_ptr(&sigstack);
syscall_raw(SYS_exit, 0, 0, 0, 0, 0, 0);
}
@@ -358,6 +358,7 @@ static void test_pkru_preserved_after_sigusr1(void)
{
struct sigaction sa;
u64 pkey_reg;
+ int ret;
/* Allow access to MPK 0 and an arbitrary set of keys */
pkey_reg = pkey_reg_restrictive_default();
@@ -369,10 +370,8 @@ static void test_pkru_preserved_after_sigusr1(void)
sa.sa_sigaction = sigusr1_handler;
sigemptyset(&sa.sa_mask);
- if (sigaction(SIGUSR1, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGUSR1, &sa, NULL);
+ pkey_assert(ret == 0);
memset(&siginfo, 0, sizeof(siginfo));
@@ -444,6 +443,7 @@ static void test_pkru_sigreturn(void)
int parent_pid = 0;
int child_pid = 0;
u64 pkey_reg;
+ int ret;
sa.sa_handler = SIG_DFL;
sa.sa_flags = 0;
@@ -453,24 +453,20 @@ static void test_pkru_sigreturn(void)
* For this testcase, we do not want to handle SIGSEGV. Reset handler
* to default so that the application can crash if it receives SIGSEGV.
*/
- if (sigaction(SIGSEGV, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGSEGV, &sa, NULL);
+ pkey_assert(ret == 0);
sa.sa_flags = SA_SIGINFO | SA_ONSTACK;
sa.sa_sigaction = sigusr2_handler;
sigemptyset(&sa.sa_mask);
- if (sigaction(SIGUSR2, &sa, NULL) == -1) {
- perror("sigaction");
- exit(EXIT_FAILURE);
- }
+ ret = sigaction(SIGUSR2, &sa, NULL);
+ pkey_assert(ret == 0);
stack = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- assert(stack != MAP_FAILED);
+ pkey_assert(stack != MAP_FAILED);
/*
* Allow access to MPK 0 and MPK 2. The child thread (to be created
@@ -494,23 +490,22 @@ static void test_pkru_sigreturn(void)
sigstack.ss_size = STACK_SIZE;
/* Use clone to avoid newer glibcs using rseq on new threads */
- long ret = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
- CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
- CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
- CLONE_DETACHED,
- stack + STACK_SIZE,
- &parent_pid,
- &child_pid);
-
- if (ret < 0) {
- errno = -ret;
- perror("clone");
- } else if (ret == 0) {
+ child_pid = clone_raw(CLONE_VM | CLONE_FS | CLONE_FILES |
+ CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM |
+ CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID |
+ CLONE_DETACHED,
+ stack + STACK_SIZE,
+ &parent_pid,
+ &child_pid);
+
+ if (child_pid < 0) {
+ errno = -child_pid;
+ pkey_assert(0);
+ } else if (child_pid == 0) {
thread_sigusr2_self(&sigstack);
syscall_raw(SYS_exit, 0, 0, 0, 0, 0, 0);
}
- child_pid = ret;
/* Check that thread exited */
do {
sched_yield();
@@ -530,16 +525,17 @@ static void (*pkey_tests[])(void) = {
int main(int argc, char *argv[])
{
- int i;
-
ksft_print_header();
ksft_set_plan(ARRAY_SIZE(pkey_tests));
if (!is_pkeys_supported())
ksft_exit_skip("pkeys not supported\n");
- for (i = 0; i < ARRAY_SIZE(pkey_tests); i++)
- (*pkey_tests[i])();
+ for (test_nr = 0; test_nr < ARRAY_SIZE(pkey_tests); test_nr++) {
+ tracing_on();
+ (*pkey_tests[test_nr])();
+ tracing_off();
+ }
ksft_finished();
return 0;
diff --git a/tools/testing/selftests/mm/pkey_util.c b/tools/testing/selftests/mm/pkey_util.c
index 255b332f7a08..a080d1dcc82d 100644
--- a/tools/testing/selftests/mm/pkey_util.c
+++ b/tools/testing/selftests/mm/pkey_util.c
@@ -2,9 +2,90 @@
#define __SANE_USERSPACE_TYPES__
#include <sys/syscall.h>
#include <unistd.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
#include "pkey-helpers.h"
+void cat_into_file(char *str, char *file)
+{
+ int fd = open(file, O_RDWR);
+ int ret;
+
+ dprintf2("%s(): writing '%s' to '%s'\n", __func__, str, file);
+ if (fd < 0) {
+ fprintf(stderr, "error opening '%s'\n", str);
+ perror("error: ");
+ exit(__LINE__);
+ }
+
+ ret = write(fd, str, strlen(str));
+ if (ret != strlen(str)) {
+ perror("write to file failed");
+ fprintf(stderr, "filename: '%s' str: '%s'\n", file, str);
+ exit(__LINE__);
+ }
+ close(fd);
+}
+
+#if CONTROL_TRACING > 0
+static int warned_tracing;
+static int tracing_root_ok(void)
+{
+ if (geteuid() != 0) {
+ if (!warned_tracing)
+ fprintf(stderr, "WARNING: not run as root, "
+ "can not do tracing control\n");
+ warned_tracing = 1;
+ return 0;
+ }
+ return 1;
+}
+#endif
+
+void tracing_on(void)
+{
+#if CONTROL_TRACING > 0
+#define TRACEDIR "/sys/kernel/tracing"
+ char pidstr[32];
+
+ if (!tracing_root_ok())
+ return;
+
+ sprintf(pidstr, "%d", getpid());
+ cat_into_file("0", TRACEDIR "/tracing_on");
+ cat_into_file("\n", TRACEDIR "/trace");
+ if (1) {
+ cat_into_file("function_graph", TRACEDIR "/current_tracer");
+ cat_into_file("1", TRACEDIR "/options/funcgraph-proc");
+ } else {
+ cat_into_file("nop", TRACEDIR "/current_tracer");
+ }
+ cat_into_file(pidstr, TRACEDIR "/set_ftrace_pid");
+ cat_into_file("1", TRACEDIR "/tracing_on");
+ dprintf1("enabled tracing\n");
+#endif
+}
+
+void tracing_off(void)
+{
+#if CONTROL_TRACING > 0
+ if (!tracing_root_ok())
+ return;
+ cat_into_file("0", "/sys/kernel/tracing/tracing_on");
+#endif
+}
+
+void abort_hooks(void)
+{
+ fprintf(stderr, "running %s()...\n", __func__);
+ tracing_off();
+#ifdef SLEEP_ON_ABORT
+ sleep(SLEEP_ON_ABORT);
+#endif
+}
+
int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
{
int ret = syscall(SYS_pkey_alloc, flags, init_val);
diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index 2085982dba69..3c0cbf8f3eaa 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -61,88 +61,6 @@ noinline int read_ptr(int *ptr)
return *ptr;
}
-static void cat_into_file(char *str, char *file)
-{
- int fd = open(file, O_RDWR);
- int ret;
-
- dprintf2("%s(): writing '%s' to '%s'\n", __func__, str, file);
- /*
- * these need to be raw because they are called under
- * pkey_assert()
- */
- if (fd < 0) {
- fprintf(stderr, "error opening '%s'\n", str);
- perror("error: ");
- exit(__LINE__);
- }
-
- ret = write(fd, str, strlen(str));
- if (ret != strlen(str)) {
- perror("write to file failed");
- fprintf(stderr, "filename: '%s' str: '%s'\n", file, str);
- exit(__LINE__);
- }
- close(fd);
-}
-
-#if CONTROL_TRACING > 0
-static int warned_tracing;
-static int tracing_root_ok(void)
-{
- if (geteuid() != 0) {
- if (!warned_tracing)
- fprintf(stderr, "WARNING: not run as root, "
- "can not do tracing control\n");
- warned_tracing = 1;
- return 0;
- }
- return 1;
-}
-#endif
-
-static void tracing_on(void)
-{
-#if CONTROL_TRACING > 0
-#define TRACEDIR "/sys/kernel/tracing"
- char pidstr[32];
-
- if (!tracing_root_ok())
- return;
-
- sprintf(pidstr, "%d", getpid());
- cat_into_file("0", TRACEDIR "/tracing_on");
- cat_into_file("\n", TRACEDIR "/trace");
- if (1) {
- cat_into_file("function_graph", TRACEDIR "/current_tracer");
- cat_into_file("1", TRACEDIR "/options/funcgraph-proc");
- } else {
- cat_into_file("nop", TRACEDIR "/current_tracer");
- }
- cat_into_file(pidstr, TRACEDIR "/set_ftrace_pid");
- cat_into_file("1", TRACEDIR "/tracing_on");
- dprintf1("enabled tracing\n");
-#endif
-}
-
-static void tracing_off(void)
-{
-#if CONTROL_TRACING > 0
- if (!tracing_root_ok())
- return;
- cat_into_file("0", "/sys/kernel/tracing/tracing_on");
-#endif
-}
-
-void abort_hooks(void)
-{
- fprintf(stderr, "running %s()...\n", __func__);
- tracing_off();
-#ifdef SLEEP_ON_ABORT
- sleep(SLEEP_ON_ABORT);
-#endif
-}
-
/*
* This attempts to have roughly a page of instructions followed by a few
* instructions that do a write, and another page of instructions. That
--
2.50.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v3 2/2] selftests/mm: add missing mmap() return checks in pkey tests
2026-05-27 1:38 [PATCH v3 0/2] selftests/mm: refactor pkey helpers and fix mmap error handling Hongfu Li
2026-05-27 1:38 ` [PATCH v3 1/2] selftests/mm: refactor pkey test helpers and unify assertions Hongfu Li
@ 2026-05-27 1:38 ` Hongfu Li
2026-05-27 11:36 ` Mike Rapoport
1 sibling, 1 reply; 5+ messages in thread
From: Hongfu Li @ 2026-05-27 1:38 UTC (permalink / raw)
To: akpm, david, ljs, liam, vbabka, rppt, surenb, mhocko, shuah
Cc: linux-mm, linux-kselftest, linux-kernel, Hongfu Li
Add missing checks against mmap() return value, replace (void *)-1
with MAP_FAILED for better readability and consistency.
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
---
tools/testing/selftests/mm/pkey-powerpc.h | 2 +-
.../testing/selftests/mm/pkey_sighandler_tests.c | 2 ++
tools/testing/selftests/mm/protection_keys.c | 16 ++++++++++------
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/mm/pkey-powerpc.h b/tools/testing/selftests/mm/pkey-powerpc.h
index 17bf2d1b0192..f482b1abcf55 100644
--- a/tools/testing/selftests/mm/pkey-powerpc.h
+++ b/tools/testing/selftests/mm/pkey-powerpc.h
@@ -126,7 +126,7 @@ static inline void *malloc_pkey_with_mprotect_subpage(long size, int prot, u16 p
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 != (void *)MAP_FAILED);
ret = syscall(__NR_subpage_prot, ptr, size, NULL);
if (ret) {
diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index 231dfb079075..475aa3e3208a 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);
+ pkey_assert(sigstack.ss_sp != MAP_FAILED);
sigstack.ss_flags = 0;
sigstack.ss_size = STACK_SIZE;
@@ -486,6 +487,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);
+ pkey_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 3c0cbf8f3eaa..ae474e65f3aa 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -585,7 +585,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);
@@ -608,7 +608,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);
@@ -688,7 +688,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);
@@ -717,7 +717,7 @@ static void *malloc_pkey(long size, int prot, u16 pkey)
pkey_assert(malloc_type < nr_malloc_types);
ret = pkey_malloc[malloc_type](size, prot, pkey);
- pkey_assert(ret != (void *)-1);
+ pkey_assert(ret != MAP_FAILED);
malloc_type++;
if (malloc_type >= nr_malloc_types)
@@ -1135,6 +1135,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 */
@@ -1693,7 +1694,10 @@ 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);
+ if (ptr == MAP_FAILED) {
+ perror("mmap");
+ return EXIT_FAILURE;
+ }
test_mprotect_pkey_on_unsupported_cpu(ptr, 1);
exit(0);
}
@@ -1706,5 +1710,5 @@ int main(void)
run_tests_once();
printf("done (all tests OK)\n");
- return 0;
+ return EXIT_SUCCESS;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 5+ messages in thread