Messages in this thread | | | Date | Fri, 10 Dec 2021 14:30:09 +0100 | From | Marco Elver <> | Subject | Re: [PATCH v4 7/7] selftests: test uaccess logging |
| |
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 <pcc@google.com>
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.
> --- > 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.c > > diff --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 += vm > diff --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.mk > diff --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 >
| |