Re: [PATCH v18 15/15] selftests, arm64: add a selftest for passing tagged... | linux-arm-kernel

quoted

Hi,

On 8/23/19 11:19 PM, Cristian Marussi wrote:
Hi

On 23/08/2019 18:16, Andrey Konovalov wrote:
On Fri, Aug 23, 2019 at 3:56 PM Cristian Marussi
[off-list ref] wrote:
Hi Andrey

On 24/06/2019 15:33, Andrey Konovalov wrote:
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.c
After 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
  endif

but 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 ?
I guess for cross-compiling it picks from cross_compiler path include
headers path without explicitly displaying it in compilation logs.
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/
Agree with Cristian that including like this is better as it allows
freedom from toolchain with latest kernel headers. Anyway there are many
places where "/sys/prctl.h" is used.

//Amit
...

Cheers

Cristian


Thanks

Cristian


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_test
diff --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.mk
diff --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]"
+fi
diff --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
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help