From: Kees Cook <keescook@chromium.org>
To: Gabriel Krisman Bertazi <krisman@collabora.com>
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org,
kernel@collabora.com, willy@infradead.org, luto@kernel.org,
gofmanp@gmail.com, linux-kselftest@vger.kernel.org,
shuah@kernel.org
Subject: Re: [PATCH v3 2/2] selftests: Add kselftest for syscall user dispatch
Date: Mon, 13 Jul 2020 17:22:42 -0700 [thread overview]
Message-ID: <202007131716.303AF8371F@keescook> (raw)
In-Reply-To: <20200712044516.2347844-3-krisman@collabora.com>
On Sun, Jul 12, 2020 at 12:45:16AM -0400, Gabriel Krisman Bertazi wrote:
> Implement functionality tests for syscall user dispatch. In order to
> make the test portable, refrain from open coding syscall dispatchers and
> calculating glibc memory ranges.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> ---
> tools/testing/selftests/Makefile | 1 +
> .../syscall_user_dispatch/.gitignore | 1 +
> .../selftests/syscall_user_dispatch/Makefile | 5 +
> .../selftests/syscall_user_dispatch/config | 1 +
> .../syscall_user_dispatch.c | 259 ++++++++++++++++++
> 5 files changed, 267 insertions(+)
> create mode 100644 tools/testing/selftests/syscall_user_dispatch/.gitignore
> create mode 100644 tools/testing/selftests/syscall_user_dispatch/Makefile
> create mode 100644 tools/testing/selftests/syscall_user_dispatch/config
> create mode 100644 tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 1195bd85af38..31b07dd774a6 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -47,6 +47,7 @@ TARGETS += openat2
> TARGETS += rseq
> TARGETS += rtc
> TARGETS += seccomp
> +TARGETS += syscall_user_dispatch
> TARGETS += sigaltstack
> TARGETS += size
> TARGETS += sparc64
nit: moar alphabetical! :)
> diff --git a/tools/testing/selftests/syscall_user_dispatch/.gitignore b/tools/testing/selftests/syscall_user_dispatch/.gitignore
> new file mode 100644
> index 000000000000..fadfb304c539
> --- /dev/null
> +++ b/tools/testing/selftests/syscall_user_dispatch/.gitignore
> @@ -0,0 +1 @@
> +syscall_user_dispatch
nit: this needs as the first line:
# SPDX-License-Identifier: GPL-2.0-only
> diff --git a/tools/testing/selftests/syscall_user_dispatch/Makefile b/tools/testing/selftests/syscall_user_dispatch/Makefile
> new file mode 100644
> index 000000000000..4785c98d4714
> --- /dev/null
> +++ b/tools/testing/selftests/syscall_user_dispatch/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +CFLAGS += -Wall
> +
> +TEST_GEN_PROGS := syscall_user_dispatch
> +include ../lib.mk
> diff --git a/tools/testing/selftests/syscall_user_dispatch/config b/tools/testing/selftests/syscall_user_dispatch/config
> new file mode 100644
> index 000000000000..22c4dfe167ca
> --- /dev/null
> +++ b/tools/testing/selftests/syscall_user_dispatch/config
> @@ -0,0 +1 @@
> +CONFIG_SYSCALL_USER_DISPATCH=y
> diff --git a/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c b/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c
> new file mode 100644
> index 000000000000..d713147863ef
> --- /dev/null
> +++ b/tools/testing/selftests/syscall_user_dispatch/syscall_user_dispatch.c
> @@ -0,0 +1,259 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020 Collabora Ltd.
> + *
> + * Test code for syscall user dispatch
> + */
> +
> +#define _GNU_SOURCE
> +#include <sys/types.h>
> +#include <sys/prctl.h>
> +#include <sys/syscall.h>
> +#include <sys/sysinfo.h>
> +#include <signal.h>
> +#include <errno.h>
> +
> +#include "../kselftest_harness.h"
> +
> +#ifndef PR_SET_SYSCALL_USER_DISPATCH
style nit: I usually use the initial ifndef to wrap all those related to
it. i.e.:
#ifndef PR_SET_SYSCALL_USER_DISPATCH
# define PR_SET_SYSCALL_USER_DISPATCH 59
# define PR_SYS_DISPATCH_OFF 0
# define PR_SYS_DISPATCH_ON 1
...
#endif
But either way is fine.
> +# define PR_SET_SYSCALL_USER_DISPATCH 59
> +#endif
> +
> +#ifndef PR_SYS_DISPATCH_OFF
> +# define PR_SYS_DISPATCH_OFF 0
> +#endif
> +
> +#ifndef PR_SYS_DISPATCH_ON
> +# define PR_SYS_DISPATCH_ON 1
> +#endif
> +
> +#ifndef SYS_USER_DISPATCH
> +# define SYS_USER_DISPATCH 2
> +#endif
> +
> +#define SYSCALL_DISPATCH_ON(x) ((x) = 1)
> +#define SYSCALL_DISPATCH_OFF(x) ((x) = 0)
> +
> +/* Test Summary:
> + *
> + * - dispatch_trigger_sigsys: Verify if PR_SET_SYSCALL_USER_DISPATCH is
> + * able to trigger SIGSYS on a syscall.
> + *
> + * - bad_selector: Test that a bad selector value triggers SIGSEGV.
> + *
> + * - bad_prctl_param: Test that the API correctly rejects invalid
> + * parameters on prctl
> + *
> + * - dispatch_and_return: Test that a syscall is selectively dispatched
> + * to userspace depending on the value of selector.
> + *
> + * - disable_dispatch: Test that the PR_SYS_DISPATCH_OFF correctly
> + * disables the dispatcher
> + *
> + * - direct_dispatch_range: Test that a syscall within the allowed range
> + * can bypass the dispatcher.
> + */
> +
> +TEST_SIGNAL(dispatch_trigger_sigsys, SIGSYS)
> +{
> + char sel = 0;
> + struct sysinfo info;
> + int ret;
> +
> + ret = sysinfo(&info);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, &sel);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH");
> + }
> +
> + SYSCALL_DISPATCH_ON(sel);
> +
> + sysinfo(&info);
> +
> + EXPECT_FALSE(true) {
> + TH_LOG("Unreachable!");
> + }
> +}
> +
> +TEST_SIGNAL(bad_selector, SIGSEGV)
> +{
> + char sel = -1;
> + long ret;
> +
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, &sel);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH");
> + }
> + EXPECT_FALSE(true) {
> + TH_LOG("Unreachable!");
> + }
> +}
> +
> +TEST(bad_prctl_param)
> +{
> + char sel = 0;
> + int op;
> +
> + /* Invalid op */
> + op = -1;
> + prctl(PR_SET_SYSCALL_USER_DISPATCH, op, 0, 0, &sel);
> + ASSERT_EQ(EINVAL, errno);
> +
> + /* PR_SYS_DISPATCH_OFF */
> + op = PR_SYS_DISPATCH_OFF;
> +
> + /* start_addr != 0 */
> + prctl(PR_SET_SYSCALL_USER_DISPATCH, op, 0x1, 0xff, 0);
> + EXPECT_EQ(EINVAL, errno);
> +
> + /* end_addr != 0 */
> + prctl(PR_SET_SYSCALL_USER_DISPATCH, op, 0x0, 0xff, 0);
> + EXPECT_EQ(EINVAL, errno);
> +
> + /* sel != NULL */
> + prctl(PR_SET_SYSCALL_USER_DISPATCH, op, 0x0, 0x0, &sel);
> + EXPECT_EQ(EINVAL, errno);
I think here would be a good place to test PR_GET_... too?
> +
> + /* Valid parameter */
> + errno = 0;
> + syscall(SYS_prctl, PR_SET_SYSCALL_USER_DISPATCH, op, 0x0, 0x0, 0x0);
> + EXPECT_EQ(0, errno);
> +
> + /* PR_SYS_DISPATCH_ON */
> + op = PR_SYS_DISPATCH_ON;
> +
> + /* start_addr > end_addr */
> + syscall(SYS_prctl, PR_SET_SYSCALL_USER_DISPATCH, op, 0x1, 0x0, &sel);
> + EXPECT_EQ(EINVAL, errno);
> +
> + /* Invalid selector */
> + syscall(SYS_prctl, PR_SET_SYSCALL_USER_DISPATCH, op, 0x0, 0x1, (void *) -1);
> + ASSERT_EQ(EFAULT, errno);
> +}
> +
> +#define MAGIC_SYSCALL_1 0xff00 /* Bad Linux syscall number */
Some archs do weird things with syscalls. Maybe use __NR_syscalls + 1?
For x86, this should be fine, though.
> +
> +/*
> + * Use global selector for handle_sigsys tests, to avoid passing
> + * selector to signal handler
> + */
> +char glob_sel;
> +int nr_syscalls_emulated;
> +int si_code;
> +
> +static void handle_sigsys(int sig, siginfo_t *info, void *ucontext)
> +{
> + si_code = info->si_code;
> +
> + if (info->si_syscall == MAGIC_SYSCALL_1)
> + nr_syscalls_emulated++;
> +
> + /* In preparation for sigreturn. */
> + SYSCALL_DISPATCH_OFF(glob_sel);
> +}
> +
> +TEST(dispatch_and_return)
> +{
> + long ret;
> + struct sigaction act;
> + sigset_t mask;
> +
> + glob_sel = 0;
> + nr_syscalls_emulated = 0;
> + si_code = 0;
> +
> + memset(&act, 0, sizeof(act));
> + sigemptyset(&mask);
> +
> + act.sa_sigaction = handle_sigsys;
> + act.sa_flags = SA_SIGINFO;
> + act.sa_mask = mask;
> +
> + ret = sigaction(SIGSYS, &act, NULL);
> + ASSERT_EQ(0, ret);
> +
> + /* Make sure selector is good prior to prctl. */
> + SYSCALL_DISPATCH_OFF(glob_sel);
> +
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, &glob_sel);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH");
> + }
> +
> + /* MAGIC_SYSCALL_1 doesn't exist. */
> + SYSCALL_DISPATCH_OFF(glob_sel);
> + ret = syscall(MAGIC_SYSCALL_1);
> + EXPECT_EQ(-1, ret) {
> + TH_LOG("Dispatch triggered unexpectedly");
> + }
> +
> + /* MAGIC_SYSCALL_1 should be emulated. */
> + nr_syscalls_emulated = 0;
> + SYSCALL_DISPATCH_ON(glob_sel);
> +
> + ret = syscall(MAGIC_SYSCALL_1);
> + EXPECT_EQ(MAGIC_SYSCALL_1, ret) {
> + TH_LOG("Failed to intercept syscall");
> + }
> + EXPECT_EQ(1, nr_syscalls_emulated) {
> + TH_LOG("Failed to emulate syscall");
> + }
> + ASSERT_EQ(SYS_USER_DISPATCH, si_code) {
> + TH_LOG("Bad si_code in SIGSYS");
> + }
> +}
> +
> +TEST(disable_dispatch)
> +{
> + int ret;
> + struct sysinfo info;
> + char sel = 0;
> +
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, 0, &sel);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH");
> + }
> +
> + /* MAGIC_SYSCALL_1 doesn't exist. */
> + SYSCALL_DISPATCH_OFF(glob_sel);
> +
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_OFF, 0, 0, 0);
> + EXPECT_EQ(0, ret) {
> + TH_LOG("Failed to unset syscall user dispatch");
> + }
> +
> + /* Shouldn't have any effect... */
> + SYSCALL_DISPATCH_ON(glob_sel);
> +
> + ret = syscall(SYS_sysinfo, &info);
> + EXPECT_EQ(0, ret) {
> + TH_LOG("Dispatch triggered unexpectedly");
> + }
> +}
> +
> +TEST(direct_dispatch_range)
> +{
> + int ret = 0;
> + struct sysinfo info;
> + char sel = 0;
> +
> + /*
> + * Instead of calculating libc addresses; allow the entire
> + * memory map and lock the selector.
> + */
> + ret = prctl(PR_SET_SYSCALL_USER_DISPATCH, PR_SYS_DISPATCH_ON, 0, -1L, &sel);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Kernel does not support CONFIG_SYSCALL_USER_DISPATCH");
> + }
> +
> + SYSCALL_DISPATCH_ON(sel);
> +
> + ret = syscall(SYS_sysinfo, &info);
> + ASSERT_EQ(0, ret) {
> + TH_LOG("Dispatch triggered unexpectedly");
> + }
> +}
> +
> +TEST_HARNESS_MAIN
Yay tests! :) Thank you! :)
--
Kees Cook
prev parent reply other threads:[~2020-07-14 0:22 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-12 4:45 [PATCH v3 0/2] Syscall user redirection Gabriel Krisman Bertazi
2020-07-12 4:45 ` [PATCH v3 1/2] kernel: Implement selective syscall userspace redirection Gabriel Krisman Bertazi
2020-07-14 0:16 ` Kees Cook
2020-07-12 4:45 ` [PATCH v3 2/2] selftests: Add kselftest for syscall user dispatch Gabriel Krisman Bertazi
2020-07-14 0:22 ` Kees Cook [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202007131716.303AF8371F@keescook \
--to=keescook@chromium.org \
--cc=gofmanp@gmail.com \
--cc=kernel@collabora.com \
--cc=krisman@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=luto@kernel.org \
--cc=shuah@kernel.org \
--cc=tglx@linutronix.de \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.