| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YbNWYSsZ7bpV13jp@elver.google.com>
Date: Fri, 10 Dec 2021 14:30:09 +0100
From: Marco Elver <elver@...gle.com>
To: Peter Collingbourne <pcc@...gle.com>
Cc: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Daniel Bristot de Oliveira <bristot@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Andy Lutomirski <luto@...nel.org>,
Kees Cook <keescook@...omium.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Masahiro Yamada <masahiroy@...nel.org>,
Sami Tolvanen <samitolvanen@...gle.com>,
YiFei Zhu <yifeifz2@...inois.edu>,
Mark Rutland <mark.rutland@....com>,
Frederic Weisbecker <frederic@...nel.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
Andrey Konovalov <andreyknvl@...il.com>,
Gabriel Krisman Bertazi <krisman@...labora.com>,
Chris Hyser <chris.hyser@...cle.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Chris Wilson <chris@...is-wilson.co.uk>,
Arnd Bergmann <arnd@...db.de>,
Dmitry Vyukov <dvyukov@...gle.com>,
Christian Brauner <christian.brauner@...ntu.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Alexey Gladkov <legion@...nel.org>,
Ran Xiaokai <ran.xiaokai@....com.cn>,
David Hildenbrand <david@...hat.com>,
Xiaofeng Cao <caoxiaofeng@...ong.com>,
Cyrill Gorcunov <gorcunov@...il.com>,
Thomas Cedeno <thomascedeno@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Evgenii Stepanov <eugenis@...gle.com>
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@...gle.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
>
Powered by blists - more mailing lists