sys_pkey_alloc, sys_pkey_free and sys_mprotect_pkey are currently
used in protections_keys.c, while pkey_sighandler_tests.c calls the
libc wrappers directly (e.g. pkey_mprotect()). This is probably ok
when using glibc (those symbols appeared a while ago), but Musl does
not currently provide them. The logging in the helpers from
pkey-helpers.h can also come in handy.
Make things more consistent by using the sys_pkey helpers in
pkey_sighandler_tests.c too. To that end their implementation is
moved to a common .c file (pkey_util.c). This also enables calling
is_pkeys_supported() outside of protections_keys.c, since it relies
on sys_pkey_{alloc,free}.
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
tools/testing/selftests/mm/Makefile | 4 +-
tools/testing/selftests/mm/pkey-helpers.h | 2 +
.../selftests/mm/pkey_sighandler_tests.c | 8 ++--
tools/testing/selftests/mm/pkey_util.c | 40 +++++++++++++++++++
tools/testing/selftests/mm/protection_keys.c | 35 ----------------
5 files changed, 48 insertions(+), 41 deletions(-)
create mode 100644 tools/testing/selftests/mm/pkey_util.c
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 814b17a43385..1f0743d9459d 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -147,8 +147,8 @@ TEST_FILES += write_hugetlb_memory.sh
include ../lib.mk
-$(TEST_GEN_PROGS): vm_util.c thp_settings.c
-$(TEST_GEN_FILES): vm_util.c thp_settings.c
+$(TEST_GEN_PROGS): vm_util.c thp_settings.c pkey_util.c
+$(TEST_GEN_FILES): vm_util.c thp_settings.c pkey_util.c
$(OUTPUT)/uffd-stress: uffd-common.c
$(OUTPUT)/uffd-unit-tests: uffd-common.c
diff --git a/tools/testing/selftests/mm/pkey-helpers.h b/tools/testing/selftests/mm/pkey-helpers.h
index 6f0ab7b42738..f080e97b39be 100644
--- a/tools/testing/selftests/mm/pkey-helpers.h
+++ b/tools/testing/selftests/mm/pkey-helpers.h
@@ -89,6 +89,8 @@ extern void abort_hooks(void);
int sys_pkey_alloc(unsigned long flags, unsigned long init_val);
int sys_pkey_free(unsigned long pkey);
+int sys_mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,
+ unsigned long pkey);
/* For functions called from protection_keys.c only */
noinline int read_ptr(int *ptr);
diff --git a/tools/testing/selftests/mm/pkey_sighandler_tests.c b/tools/testing/selftests/mm/pkey_sighandler_tests.c
index e1aaeb65cfae..c73cee192b88 100644
--- a/tools/testing/selftests/mm/pkey_sighandler_tests.c
+++ b/tools/testing/selftests/mm/pkey_sighandler_tests.c
@@ -311,8 +311,8 @@ static void test_sigsegv_handler_with_different_pkey_for_stack(void)
__write_pkey_reg(pkey_reg);
/* Protect the new stack with MPK 1 */
- pkey = pkey_alloc(0, 0);
- pkey_mprotect(stack, STACK_SIZE, PROT_READ | PROT_WRITE, pkey);
+ pkey = sys_pkey_alloc(0, 0);
+ sys_mprotect_pkey(stack, STACK_SIZE, PROT_READ | PROT_WRITE, pkey);
/* Set up alternate signal stack that will use the default MPK */
sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC,
@@ -484,8 +484,8 @@ static void test_pkru_sigreturn(void)
__write_pkey_reg(pkey_reg);
/* Protect the stack with MPK 2 */
- pkey = pkey_alloc(0, 0);
- pkey_mprotect(stack, STACK_SIZE, PROT_READ | PROT_WRITE, pkey);
+ pkey = sys_pkey_alloc(0, 0);
+ sys_mprotect_pkey(stack, STACK_SIZE, PROT_READ | PROT_WRITE, pkey);
/* Set up alternate signal stack that will use the default MPK */
sigstack.ss_sp = mmap(0, STACK_SIZE, PROT_READ | PROT_WRITE | PROT_EXEC,
diff --git a/tools/testing/selftests/mm/pkey_util.c b/tools/testing/selftests/mm/pkey_util.c
new file mode 100644
index 000000000000..ca4ad0d44ab2
--- /dev/null
+++ b/tools/testing/selftests/mm/pkey_util.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "pkey-helpers.h"
+
+int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
+{
+ int ret = syscall(SYS_pkey_alloc, flags, init_val);
+ dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
+ __func__, flags, init_val, ret, errno);
+ return ret;
+}
+
+int sys_pkey_free(unsigned long pkey)
+{
+ int ret = syscall(SYS_pkey_free, pkey);
+ dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
+ return ret;
+}
+
+int sys_mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,
+ unsigned long pkey)
+{
+ int sret;
+
+ dprintf2("%s(0x%p, %zx, prot=%lx, pkey=%lx)\n", __func__,
+ ptr, size, orig_prot, pkey);
+
+ errno = 0;
+ sret = syscall(__NR_pkey_mprotect, ptr, size, orig_prot, pkey);
+ if (errno) {
+ dprintf2("SYS_mprotect_key sret: %d\n", sret);
+ dprintf2("SYS_mprotect_key prot: 0x%lx\n", orig_prot);
+ dprintf2("SYS_mprotect_key failed, errno: %d\n", errno);
+ if (DEBUG_LEVEL >= 2)
+ perror("SYS_mprotect_pkey");
+ }
+ return sret;
+}
diff --git a/tools/testing/selftests/mm/protection_keys.c b/tools/testing/selftests/mm/protection_keys.c
index f43cf3b75d8e..3688571e6b39 100644
--- a/tools/testing/selftests/mm/protection_keys.c
+++ b/tools/testing/selftests/mm/protection_keys.c
@@ -460,34 +460,6 @@ static pid_t fork_lazy_child(void)
return forkret;
}
-int sys_mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,
- unsigned long pkey)
-{
- int sret;
-
- dprintf2("%s(0x%p, %zx, prot=%lx, pkey=%lx)\n", __func__,
- ptr, size, orig_prot, pkey);
-
- errno = 0;
- sret = syscall(__NR_pkey_mprotect, ptr, size, orig_prot, pkey);
- if (errno) {
- dprintf2("SYS_mprotect_key sret: %d\n", sret);
- dprintf2("SYS_mprotect_key prot: 0x%lx\n", orig_prot);
- dprintf2("SYS_mprotect_key failed, errno: %d\n", errno);
- if (DEBUG_LEVEL >= 2)
- perror("SYS_mprotect_pkey");
- }
- return sret;
-}
-
-int sys_pkey_alloc(unsigned long flags, unsigned long init_val)
-{
- int ret = syscall(SYS_pkey_alloc, flags, init_val);
- dprintf1("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n",
- __func__, flags, init_val, ret, errno);
- return ret;
-}
-
static int alloc_pkey(void)
{
int ret;
@@ -534,13 +506,6 @@ static int alloc_pkey(void)
return ret;
}
-int sys_pkey_free(unsigned long pkey)
-{
- int ret = syscall(SYS_pkey_free, pkey);
- dprintf1("%s(pkey=%ld) syscall ret: %d\n", __func__, pkey, ret);
- return ret;
-}
-
/*
* I had a bug where pkey bits could be set by mprotect() but
* not cleared. This ensures we get lots of random bit sets
--
2.47.0On Mon, Dec 09, 2024 at 09:50:16AM +0000, Kevin Brodsky wrote:
Hi Kevin, Andrew,
> diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
> index 814b17a43385..1f0743d9459d 100644
> --- a/tools/testing/selftests/mm/Makefile
> +++ b/tools/testing/selftests/mm/Makefile
> @@ -147,8 +147,8 @@ TEST_FILES += write_hugetlb_memory.sh
>
> include ../lib.mk
>
> -$(TEST_GEN_PROGS): vm_util.c thp_settings.c
> -$(TEST_GEN_FILES): vm_util.c thp_settings.c
> +$(TEST_GEN_PROGS): vm_util.c thp_settings.c pkey_util.c
> +$(TEST_GEN_FILES): vm_util.c thp_settings.c pkey_util.c
This patch breaks s390 in -next with the below error messages:
In file included from pkey_util.c:5:
pkey-helpers.h:109:2: error: #error Architecture not supported
109 | #error Architecture not supported
| ^~~~~
pkey-helpers.h: In function ‘set_pkey_bits’:
pkey-helpers.h:126:21: error: implicit declaration of function ‘pkey_bit_position’ [-Wimplicit-function-declaration]
126 | u32 shift = pkey_bit_position(pkey);
| ^~~~~~~~~~~~~~~~~
pkey-helpers.h: In function ‘_read_pkey_reg’:
pkey-helpers.h:151:24: error: implicit declaration of function ‘__read_pkey_reg’; did you mean ‘_read_pkey_reg’? [-Wimplicit-function-declaration]
151 | u64 pkey_reg = __read_pkey_reg();
| ^~~~~~~~~~~~~~~
| _read_pkey_reg
pkey-helpers.h: In function ‘write_pkey_reg’:
pkey-helpers.h:165:18: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘int’ [-Wformat=]
165 | dprintf4("%s() changing %016llx to %016llx\n", __func__,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
166 | __read_pkey_reg(), pkey_reg);
| ~~~~~~~~~~~~~~~~~
| |
| int
pkey-helpers.h:62:32: note: in definition of macro ‘dprintf_level’
62 | sigsafe_printf(args); \
| ^~~~
pkey-helpers.h:165:9: note: in expansion of macro ‘dprintf4’
165 | dprintf4("%s() changing %016llx to %016llx\n", __func__,
| ^~~~~~~~
pkey-helpers.h:165:39: note: format string is defined here
165 | dprintf4("%s() changing %016llx to %016llx\n", __func__,
| ~~~~~~^
| |
| long long unsigned int
| %016x
pkey-helpers.h:169:9: error: implicit declaration of function ‘__write_pkey_reg’; did you mean ‘write_pkey_reg’? [-Wimplicit-function-declaration]
169 | __write_pkey_reg(pkey_reg);
| ^~~~~~~~~~~~~~~~
| write_pkey_reg
pkey-helpers.h:171:18: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘int’ [-Wformat=]
171 | dprintf4("%s(%016llx) pkey_reg: %016llx\n", __func__,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
172 | pkey_reg, __read_pkey_reg());
| ~~~~~~~~~~~~~~~~~
| |
| int
pkey-helpers.h:62:32: note: in definition of macro ‘dprintf_level’
62 | sigsafe_printf(args); \
| ^~~~
pkey-helpers.h:171:9: note: in expansion of macro ‘dprintf4’
171 | dprintf4("%s(%016llx) pkey_reg: %016llx\n", __func__,
| ^~~~~~~~
pkey-helpers.h:171:47: note: format string is defined here
171 | dprintf4("%s(%016llx) pkey_reg: %016llx\n", __func__,
| ~~~~~~^
| |
| long long unsigned int
| %016x
pkey-helpers.h: In function ‘is_pkeys_supported’:
pkey-helpers.h:207:14: error: implicit declaration of function ‘cpu_has_pkeys’; did you mean ‘kernel_has_pkeys’? [-Wimplicit-function-declaration]
207 | if (!cpu_has_pkeys()) {
| ^~~~~~~~~~~~~
| kernel_has_pkeys
Please, let me know if I am missing something.
> $(OUTPUT)/uffd-stress: uffd-common.c
> $(OUTPUT)/uffd-unit-tests: uffd-common.c
Thanks!
The pkey* files can only be built on architectures that support
pkeys (pkey-helpers.h #error's otherwise). Adding pkey_util.c as
dependency to all $(TEST_GEN_FILES) is therefore a bad idea. Make it
a dependency of the pkeys tests only.
Those tests are built in 32/64-bit variants on x86_64 so we need to
add an explicit dependency there as well.
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
Hi Alexander,
Thank you for the bug report, that patch indeed breaks all
architectures that don't support pkeys, I should have realised that!
This patch should fix it.
Andrew, it would make sense to squash this patch into the original
("selftests/mm: Use sys_pkey helpers consistently").
Cheers,
- Kevin
Cc: akpm@linux-foundation.org
Cc: aruna.ramakrishna@oracle.com
Cc: catalin.marinas@arm.com
Cc: dave.hansen@linux.intel.com
Cc: joey.gouly@arm.com
Cc: keith.lucas@oracle.com
Cc: ryan.roberts@arm.com
Cc: shuah@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kselftest@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Cc: linux-s390@vger.kernel.org
---
tools/testing/selftests/mm/Makefile | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index 1f0743d9459d..18041de1aebf 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -147,16 +147,20 @@ TEST_FILES += write_hugetlb_memory.sh
include ../lib.mk
-$(TEST_GEN_PROGS): vm_util.c thp_settings.c pkey_util.c
-$(TEST_GEN_FILES): vm_util.c thp_settings.c pkey_util.c
+$(TEST_GEN_PROGS): vm_util.c thp_settings.c
+$(TEST_GEN_FILES): vm_util.c thp_settings.c
$(OUTPUT)/uffd-stress: uffd-common.c
$(OUTPUT)/uffd-unit-tests: uffd-common.c
+$(OUTPUT)/protection_keys: pkey_util.c
+$(OUTPUT)/pkey_sighandler_tests: pkey_util.c
ifeq ($(ARCH),x86_64)
BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
+$(BINARIES_32) $(BINARIES_64): pkey_util.c
+
define gen-target-rule-32
$(1) $(1)_32: $(OUTPUT)/$(1)_32
.PHONY: $(1) $(1)_32
--
2.47.0© 2016 - 2025 Red Hat, Inc.