Re: [PATCH v4 7/7] selftests: test uaccess logging
From: Marco Elver <elver@google.com>
Date: 2021-12-10 13:30:24
Also in:
lkml
On Thu, Dec 09, 2021 at 02:15PM -0800, Peter Collingbourne wrote:
Add a kselftest for the uaccess logging feature. Link: https://linux-review.googlesource.com/id/I39e1707fb8aef53747c42bd55b46ecaa67205199 Signed-off-by: Peter Collingbourne <redacted>
It would be good to also test: - Logging of reads. - Exhausting the uaccess buffer, ideally somehow checking that the kernel hasn't written out-of-bounds, e.g. by using some canary. - Passing an invalid address to some syscall, for which the access should not be logged? - Passing an invalid address to the PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR prctl(). - Passing a valid address to the prctl(), but that address points to an invalid address.
quoted hunk ↗ jump to hunk
--- tools/testing/selftests/Makefile | 1 + .../testing/selftests/uaccess_buffer/Makefile | 4 + .../uaccess_buffer/uaccess_buffer_test.c | 126 ++++++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 tools/testing/selftests/uaccess_buffer/Makefile create mode 100644 tools/testing/selftests/uaccess_buffer/uaccess_buffer_test.cdiff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c852eb40c4f7..291b62430557 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile@@ -71,6 +71,7 @@ TARGETS += timers endif TARGETS += tmpfs TARGETS += tpm2 +TARGETS += uaccess_buffer TARGETS += user TARGETS += vDSO TARGETS += vmdiff --git a/tools/testing/selftests/uaccess_buffer/Makefile b/tools/testing/selftests/uaccess_buffer/Makefile new file mode 100644 index 000000000000..e6e5fb43ce29 --- /dev/null +++ b/tools/testing/selftests/uaccess_buffer/Makefile@@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +TEST_GEN_PROGS := uaccess_buffer_test + +include ../lib.mkdiff --git a/tools/testing/selftests/uaccess_buffer/uaccess_buffer_test.c b/tools/testing/selftests/uaccess_buffer/uaccess_buffer_test.c new file mode 100644 index 000000000000..051062e4fbf9 --- /dev/null +++ b/tools/testing/selftests/uaccess_buffer/uaccess_buffer_test.c@@ -0,0 +1,126 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "../kselftest_harness.h" + +#include <linux/uaccess-buffer.h> +#include <sys/prctl.h> +#include <sys/utsname.h> + +FIXTURE(uaccess_buffer) +{ + uint64_t addr; +}; + +FIXTURE_SETUP(uaccess_buffer) +{ + ASSERT_EQ(0, prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR, &self->addr, 0, + 0, 0)); +} + +FIXTURE_TEARDOWN(uaccess_buffer) +{ + ASSERT_EQ(0, prctl(PR_SET_UACCESS_DESCRIPTOR_ADDR_ADDR, 0, 0, 0, 0)); +} + +TEST_F(uaccess_buffer, uname) +{ + struct uaccess_descriptor desc; + struct uaccess_buffer_entry entries[64]; + struct utsname un; + + desc.addr = (uint64_t)(unsigned long)entries; + desc.size = 64; + self->addr = (uint64_t)(unsigned long)&desc; + ASSERT_EQ(0, uname(&un)); + ASSERT_EQ(0, self->addr); + + if (desc.size == 63) { + ASSERT_EQ((uint64_t)(unsigned long)(entries + 1), desc.addr); + + ASSERT_EQ((uint64_t)(unsigned long)&un, entries[0].addr); + ASSERT_EQ(sizeof(struct utsname), entries[0].size); + ASSERT_EQ(UACCESS_BUFFER_FLAG_WRITE, entries[0].flags); + } else { + /* See override_architecture in kernel/sys.c */ + ASSERT_EQ(62, desc.size); + ASSERT_EQ((uint64_t)(unsigned long)(entries + 2), desc.addr); + + ASSERT_EQ((uint64_t)(unsigned long)&un, entries[0].addr); + ASSERT_EQ(sizeof(struct utsname), entries[0].size); + ASSERT_EQ(UACCESS_BUFFER_FLAG_WRITE, entries[0].flags); + + ASSERT_EQ((uint64_t)(unsigned long)&un.machine, + entries[1].addr); + ASSERT_EQ(UACCESS_BUFFER_FLAG_WRITE, entries[1].flags); + } +} + +static bool handled; + +static void usr1_handler(int signo) +{ + handled = true; +} + +TEST_F(uaccess_buffer, blocked_signals) +{ + struct uaccess_descriptor desc; + struct shared_buf { + bool ready; + bool killed; + } volatile *shared = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE, + MAP_ANON | MAP_SHARED, -1, 0);
I know it's a synonym, but to be consistent with other code, MAP_ANONYMOUS?
+ struct sigaction act = {}, oldact;
+ int pid;
+
+ handled = false;
+ act.sa_handler = usr1_handler;
+ sigaction(SIGUSR1, &act, &oldact);
+
+ pid = fork();
+ if (pid == 0) {
+ /*
+ * Busy loop to synchronize instead of issuing syscalls because
+ * we need to test the behavior in the case where no syscall is
+ * issued by the parent process.
+ */
+ while (!shared->ready)
+ ;
+ kill(getppid(), SIGUSR1);
+ shared->killed = true;
+ _exit(0);
+ } else {
+ int i;
+
+ desc.addr = 0;
+ desc.size = 0;
+ self->addr = (uint64_t)(unsigned long)&desc;
+
+ shared->ready = true;
+ while (!shared->killed)
+ ;
+
+ /*
+ * The kernel should have IPI'd us by now, but let's wait a bit
+ * longer just in case.Is IPI = signalled? Because in the kernel, IPI = inter-processor interrupt.
+ */ + for (i = 0; i != 1000000; ++i) + ;
This is probably optimized out. usleep() should work, or add compiler barrier if usleep doesn't work.
+ + ASSERT_FALSE(handled); + + /* + * Returning from the waitpid syscall should trigger the signal + * handler. The signal itself may also interrupt waitpid, so + * make sure to handle EINTR. + */ + while (waitpid(pid, NULL, 0) == -1) + ASSERT_EQ(EINTR, errno); + ASSERT_TRUE(handled); + } + + munmap((void *)shared, getpagesize()); + sigaction(SIGUSR1, &oldact, NULL); +} + +TEST_HARNESS_MAIN -- 2.34.1.173.g76aa8bc2d0-goog
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel