| Message ID | 20211209221545.2333249-8-pcc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kernel: introduce uaccess logging | expand |
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 >
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); + 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. + */ + for (i = 0; i != 1000000; ++i) + ; + + 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
Add a kselftest for the uaccess logging feature. Link: https://linux-review.googlesource.com/id/I39e1707fb8aef53747c42bd55b46ecaa67205199 Signed-off-by: Peter Collingbourne <pcc@google.com> --- 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