Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged pointers to kernel
From: Andrey Konovalov <hidden>
Date: 2019-09-04 16:42:59
Also in:
amd-gfx, dri-devel, kvm, linux-kselftest, linux-media, linux-mm, linux-rdma, lkml
On Wed, Sep 4, 2019 at 6:22 PM Cristian Marussi [off-list ref] wrote:
Hi Andrey ! On 04/09/2019 15:52, Andrey Konovalov wrote:quoted
On Fri, Aug 23, 2019 at 7:49 PM Cristian Marussi [off-list ref] wrote:quoted
Hi On 23/08/2019 18:16, Andrey Konovalov wrote:quoted
On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi [off-list ref] wrote:quoted
Hi Andrey On 24/06/2019 15:33, Andrey Konovalov wrote:quoted
This patch is a part of a series that extends kernel ABI to allow to pass tagged user pointers (with the top byte set to something else other than 0x00) as syscall arguments. This patch adds a simple test, that calls the uname syscall with a tagged user pointer as an argument. Without the kernel accepting tagged user pointers the test fails with EFAULT. Signed-off-by: Andrey Konovalov <redacted> --- tools/testing/selftests/arm64/.gitignore | 1 + tools/testing/selftests/arm64/Makefile | 11 +++++++ .../testing/selftests/arm64/run_tags_test.sh | 12 ++++++++ tools/testing/selftests/arm64/tags_test.c | 29 +++++++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 tools/testing/selftests/arm64/.gitignore create mode 100644 tools/testing/selftests/arm64/Makefile create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh create mode 100644 tools/testing/selftests/arm64/tags_test.cAfter building a fresh Kernel from arm64/for-next-core from scratch at: commit 239ab658bea3b387424501e7c416640d6752dc0c Merge: 6bfa3134bd3a 42d038c4fb00 1243cb6a676f d55c5f28afaf d06fa5a118f1 34b5560db40d Author: Will Deacon [off-list ref] Date: Thu Aug 22 18:23:53 2019 +0100 Merge branches 'for-next/error-injection', 'for-next/tbi', 'for-next/psci-cpuidle', 'for-next/cpu-topology' and 'for-next/52-bit-kva' into for-next/core KSFT arm64 tests build is broken for me, both setting or not KBUILD_OUTPUT= 13:30 $ make TARGETS=arm64 kselftest-clean make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' rm -f -r /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' ✔ ~/ARM/dev/src/pdsw/linux [arm64_for_next_core|…8⚑ 23] 13:30 $ make TARGETS=arm64 kselftest make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ ARCH=arm64 -C ../../.. headers_install HOSTCC scripts/basic/fixdep HOSTCC scripts/unifdef ... ... HDRINST usr/include/asm/msgbuf.h HDRINST usr/include/asm/shmbuf.h INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/usr/include /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test tags_test.c: In function ‘main’: tags_test.c:21:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^~~~~~~~~~~~~~~~~~~~~~~ PR_GET_TID_ADDRESS tags_test.c:21:12: note: each undeclared identifier is reported only once for each function it appears in tags_test.c:21:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^~~~~~~~~~~~~~~~~~~~~ PR_GET_DUMPABLE ../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test' failed make[3]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux//kselftest/arm64/tags_test] Error 1 Makefile:136: recipe for target 'all' failed make[2]: *** [all] Error 2 /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1237: recipe for target 'kselftest' failed make[1]: *** [kselftest] Error 2 make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' Makefile:179: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 Despite seeing KSFT installing Kernel Headers, they cannot be found. Fixing this patch like this make it work for me:diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile index a61b2e743e99..f9f79fb272f0 100644 --- a/tools/testing/selftests/arm64/Makefile +++ b/tools/testing/selftests/arm64/Makefile@@ -4,6 +4,7 @@ ARCH ?= $(shell uname -m 2>/dev/null || echo not) ifneq (,$(filter $(ARCH),aarch64 arm64)) +CFLAGS += -I../../../../usr/include/ TEST_GEN_PROGS := tags_test TEST_PROGS := run_tags_test.sh endifbut is not really a proper fix since it does NOT account for case in which you have installed the Kernel Headers in a non standard location like when you use KBUILD_OUTPUT. Am I missing something ?Hm, PR_SET_TAGGED_ADDR_CTRL is defined in include/uapi/linux/prctl.h, and the test has #include <sys/prctl.h> so as long as you've updated your kernel headers this should work. (I'm OOO next week, I'll see if I can reproduce this once I'm back).Ok. Thanks for the reply. I think I've got it in my local tree having cloned arm64/for-next-core: 18:32 $ egrep -A 10 PR_SET_TAG ./include/uapi/linux/prctl.h #define PR_SET_TAGGED_ADDR_CTRL 55 #define PR_GET_TAGGED_ADDR_CTRL 56 # define PR_TAGGED_ADDR_ENABLE (1UL << 0) #endif /* _LINUX_PRCTL_H */ and Kernel header are locally installed in my kernel src dir (by KSFT indeed) 18:34 $ egrep -RA 10 PR_SET_TAG usr/include/ usr/include/linux/prctl.h:#define PR_SET_TAGGED_ADDR_CTRL 55 usr/include/linux/prctl.h-#define PR_GET_TAGGED_ADDR_CTRL 56 usr/include/linux/prctl.h-# define PR_TAGGED_ADDR_ENABLE (1UL << 0) usr/include/linux/prctl.h- usr/include/linux/prctl.h-#endif /* _LINUX_PRCTL_H */ but how are they supposed to be found if nor the test Makefile neither the KSFT Makefile who installs them pass any -I options to the compiler ? I suppose <sys/prctl.h> tries to include arch specific headers from the regular system path, but when you are cross-compiling ? 18:34 $ make TARGETS=arm64 kselftest make[1]: Entering directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' arch/arm64/Makefile:56: CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \ ARCH=arm64 -C ../../.. headers_install INSTALL /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/usr/include /opt/toolchains/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -Wall -O2 -g tags_test.c -o /home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test tags_test.c: In function ‘main’: tags_test.c:20:12: error: ‘PR_SET_TAGGED_ADDR_CTRL’ undeclared (first use in this function); did you mean ‘PR_GET_TID_ADDRESS’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^~~~~~~~~~~~~~~~~~~~~~~ PR_GET_TID_ADDRESS tags_test.c:20:12: note: each undeclared identifier is reported only once for each function it appears in tags_test.c:20:37: error: ‘PR_TAGGED_ADDR_ENABLE’ undeclared (first use in this function); did you mean ‘PR_GET_DUMPABLE’? if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) ^~~~~~~~~~~~~~~~~~~~~ PR_GET_DUMPABLE ../../lib.mk:138: recipe for target '/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test' failed make[4]: *** [/home/crimar01/ARM/dev/src/pdsw/out_linux/kselftest/arm64/tags/tags_test] Error 1 Makefile:19: recipe for target 'all' failed make[3]: *** [all] Error 2 Makefile:137: recipe for target 'all' failed make[2]: *** [all] Error 2 /home/crimar01/ARM/dev/src/pdsw/linux/Makefile:1236: recipe for target 'kselftest' failed make[1]: *** [kselftest] Error 2 make[1]: Leaving directory '/home/crimar01/ARM/dev/src/pdsw/out_linux' Makefile:179: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 In fact many KSFT testcases seems to brutally add default headers path: tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/uapi/ tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../include/ tools/testing/selftests/memfd/Makefile:CFLAGS += -I../../../../usr/include/ tools/testing/selftests/net/Makefile:CFLAGS += -I../../../../usr/include/ tools/testing/selftests/membarrier/Makefile:CFLAGS += -g -I../../../../usr/include/ ...Hi Cristian! Indeed, I can reproduce the issue. I don't know what's the proper way to resolve this. Adding "CFLAGS += -I../../../../usr/include/" looks good to me. AFAICS your series resolves this issue in a similar way, but I think we should fix this before the current rc is released. Do you want to submit a patch that adds this simple fix or should I do that?Please feel free to post the single line patch above to quickly fix this before release, so we don't have a broken build straight away. (our CI is already beating me...:D)
Done.
On my side (01/11) in the meantime I'll fix the top level KSFT arm64 makefile so as to calculate and propagate once for all the headers search path down to all KSFT arm64/ in one go, trying to guess where they are; this is needed because the above fix works fine as long as you don't have KBUILD_OUTPUT set, once you set it, KSFT installs kheaders in a different place and the above -I fix is fooled again....but this is a general problem also in other KSFT tests as I can see now so I think this fix is good enough for now (and the fix on my side, even if trivial, is not going to go into this release for sure)
Ah, I see. Thanks for the report!
Thanks ! Cheers Cristianquoted
Thanks!quoted
Cheers Cristianquoted
quoted
Thanks Cristianquoted
diff --git a/tools/testing/selftests/arm64/.gitignore b/tools/testing/selftests/arm64/.gitignore new file mode 100644 index 000000000000..e8fae8d61ed6 --- /dev/null +++ b/tools/testing/selftests/arm64/.gitignore@@ -0,0 +1 @@ +tags_testdiff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile new file mode 100644 index 000000000000..a61b2e743e99 --- /dev/null +++ b/tools/testing/selftests/arm64/Makefile@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0 + +# ARCH can be overridden by the user for cross compiling +ARCH ?= $(shell uname -m 2>/dev/null || echo not) + +ifneq (,$(filter $(ARCH),aarch64 arm64)) +TEST_GEN_PROGS := tags_test +TEST_PROGS := run_tags_test.sh +endif + +include ../lib.mkdiff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/run_tags_test.sh new file mode 100755 index 000000000000..745f11379930 --- /dev/null +++ b/tools/testing/selftests/arm64/run_tags_test.sh@@ -0,0 +1,12 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +echo "--------------------" +echo "running tags test" +echo "--------------------" +./tags_test +if [ $? -ne 0 ]; then + echo "[FAIL]" +else + echo "[PASS]" +fidiff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags_test.c new file mode 100644 index 000000000000..22a1b266e373 --- /dev/null +++ b/tools/testing/selftests/arm64/tags_test.c@@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <stdint.h> +#include <sys/prctl.h> +#include <sys/utsname.h> + +#define SHIFT_TAG(tag) ((uint64_t)(tag) << 56) +#define SET_TAG(ptr, tag) (((uint64_t)(ptr) & ~SHIFT_TAG(0xff)) | \ + SHIFT_TAG(tag)) + +int main(void) +{ + static int tbi_enabled = 0; + struct utsname *ptr, *tagged_ptr; + int err; + + if (prctl(PR_SET_TAGGED_ADDR_CTRL, PR_TAGGED_ADDR_ENABLE, 0, 0, 0) == 0) + tbi_enabled = 1; + ptr = (struct utsname *)malloc(sizeof(*ptr)); + if (tbi_enabled) + tagged_ptr = (struct utsname *)SET_TAG(ptr, 0x42); + err = uname(tagged_ptr); + free(ptr); + + return err; +}
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel