2016-03-17 09:27:34

by Matt Redfearn

[permalink] [raw]
Subject: [PATCH] selftests/seccomp: add MIPS self-test support

This adds self-test support on MIPS, based on RFC patch from Kees Cook.
Modifications from the RFC:
- support the O32 syscall which passes the real syscall number in a0.
- Use PTRACE_{GET,SET}REGS
- Because SYSCALL_NUM and SYSCALL_RET are the same register, it is not
possible to test modifying the syscall return value when skipping,
since both would need to set the same register. Therefore modify that
test case to just detect the skipped test.
Tested on MIPS32r2 with an O32 userland where 48/48 tests now pass.

Signed-off-by: Matt Redfearn <[email protected]>
---
tools/testing/selftests/seccomp/seccomp_bpf.c | 30 +++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index b9453b838162..92862c828c4a 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -5,6 +5,7 @@
* Test code for seccomp bpf.
*/

+#include <sys/types.h>
#include <asm/siginfo.h>
#define __have_siginfo_t 1
#define __have_sigval_t 1
@@ -14,7 +15,6 @@
#include <linux/filter.h>
#include <sys/prctl.h>
#include <sys/ptrace.h>
-#include <sys/types.h>
#include <sys/user.h>
#include <linux/prctl.h>
#include <linux/ptrace.h>
@@ -1242,6 +1242,12 @@ TEST_F(TRACE_poke, getpid_runs_normally)
# define ARCH_REGS s390_regs
# define SYSCALL_NUM gprs[2]
# define SYSCALL_RET gprs[2]
+#elif defined(__mips__)
+# define ARCH_REGS struct pt_regs
+# define SYSCALL_NUM regs[2]
+# define SYSCALL_SYSCALL_NUM regs[4]
+# define SYSCALL_RET regs[2]
+# define SYSCALL_NUM_RET_SHARE_REG
#else
# error "Do not know how to find your architecture's registers and syscalls"
#endif
@@ -1249,7 +1255,7 @@ TEST_F(TRACE_poke, getpid_runs_normally)
/* Use PTRACE_GETREGS and PTRACE_SETREGS when available. This is useful for
* architectures without HAVE_ARCH_TRACEHOOK (e.g. User-mode Linux).
*/
-#if defined(__x86_64__) || defined(__i386__)
+#if defined(__x86_64__) || defined(__i386__) || defined(__mips__)
#define HAVE_GETREGS
#endif

@@ -1273,6 +1279,10 @@ int get_syscall(struct __test_metadata *_metadata, pid_t tracee)
}
#endif

+#if defined(__mips__)
+ if (regs.SYSCALL_NUM == __NR_syscall)
+ return regs.SYSCALL_SYSCALL_NUM;
+#endif
return regs.SYSCALL_NUM;
}

@@ -1297,6 +1307,13 @@ void change_syscall(struct __test_metadata *_metadata,
{
regs.SYSCALL_NUM = syscall;
}
+#elif defined(__mips__)
+ {
+ if (regs.SYSCALL_NUM == __NR_syscall)
+ regs.SYSCALL_SYSCALL_NUM = syscall;
+ else
+ regs.SYSCALL_NUM = syscall;
+ }

#elif defined(__arm__)
# ifndef PTRACE_SET_SYSCALL
@@ -1327,7 +1344,11 @@ void change_syscall(struct __test_metadata *_metadata,

/* If syscall is skipped, change return value. */
if (syscall == -1)
+#ifdef SYSCALL_NUM_RET_SHARE_REG
+ TH_LOG("Can't modify syscall return on this architecture");
+#else
regs.SYSCALL_RET = 1;
+#endif

#ifdef HAVE_GETREGS
ret = ptrace(PTRACE_SETREGS, tracee, 0, &regs);
@@ -1465,8 +1486,13 @@ TEST_F(TRACE_syscall, syscall_dropped)
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
ASSERT_EQ(0, ret);

+#ifdef SYSCALL_NUM_RET_SHARE_REG
+ /* gettid has been skipped */
+ EXPECT_EQ(-1, syscall(__NR_gettid));
+#else
/* gettid has been skipped and an altered return value stored. */
EXPECT_EQ(1, syscall(__NR_gettid));
+#endif
EXPECT_NE(self->mytid, syscall(__NR_gettid));
}

--
2.5.0


2016-03-21 19:46:18

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] selftests/seccomp: add MIPS self-test support

On Thu, Mar 17, 2016 at 2:26 AM, Matt Redfearn <[email protected]> wrote:
> This adds self-test support on MIPS, based on RFC patch from Kees Cook.
> Modifications from the RFC:
> - support the O32 syscall which passes the real syscall number in a0.
> - Use PTRACE_{GET,SET}REGS
> - Because SYSCALL_NUM and SYSCALL_RET are the same register, it is not
> possible to test modifying the syscall return value when skipping,
> since both would need to set the same register. Therefore modify that
> test case to just detect the skipped test.

I wonder how s390 deals with sharing the register? Seems like it
should be failing the same test in the same way.

> Tested on MIPS32r2 with an O32 userland where 48/48 tests now pass.
>
> Signed-off-by: Matt Redfearn <[email protected]>
> ---
> tools/testing/selftests/seccomp/seccomp_bpf.c | 30 +++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index b9453b838162..92862c828c4a 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -5,6 +5,7 @@
> * Test code for seccomp bpf.
> */
>
> +#include <sys/types.h>
> #include <asm/siginfo.h>
> #define __have_siginfo_t 1
> #define __have_sigval_t 1
> @@ -14,7 +15,6 @@
> #include <linux/filter.h>
> #include <sys/prctl.h>
> #include <sys/ptrace.h>
> -#include <sys/types.h>
> #include <sys/user.h>
> #include <linux/prctl.h>
> #include <linux/ptrace.h>
> @@ -1242,6 +1242,12 @@ TEST_F(TRACE_poke, getpid_runs_normally)
> # define ARCH_REGS s390_regs
> # define SYSCALL_NUM gprs[2]
> # define SYSCALL_RET gprs[2]
> +#elif defined(__mips__)
> +# define ARCH_REGS struct pt_regs
> +# define SYSCALL_NUM regs[2]
> +# define SYSCALL_SYSCALL_NUM regs[4]
> +# define SYSCALL_RET regs[2]
> +# define SYSCALL_NUM_RET_SHARE_REG
> #else
> # error "Do not know how to find your architecture's registers and syscalls"
> #endif
> @@ -1249,7 +1255,7 @@ TEST_F(TRACE_poke, getpid_runs_normally)
> /* Use PTRACE_GETREGS and PTRACE_SETREGS when available. This is useful for
> * architectures without HAVE_ARCH_TRACEHOOK (e.g. User-mode Linux).
> */
> -#if defined(__x86_64__) || defined(__i386__)
> +#if defined(__x86_64__) || defined(__i386__) || defined(__mips__)
> #define HAVE_GETREGS
> #endif
>
> @@ -1273,6 +1279,10 @@ int get_syscall(struct __test_metadata *_metadata, pid_t tracee)
> }
> #endif
>
> +#if defined(__mips__)
> + if (regs.SYSCALL_NUM == __NR_syscall)
> + return regs.SYSCALL_SYSCALL_NUM;
> +#endif
> return regs.SYSCALL_NUM;
> }
>
> @@ -1297,6 +1307,13 @@ void change_syscall(struct __test_metadata *_metadata,
> {
> regs.SYSCALL_NUM = syscall;
> }
> +#elif defined(__mips__)
> + {
> + if (regs.SYSCALL_NUM == __NR_syscall)
> + regs.SYSCALL_SYSCALL_NUM = syscall;
> + else
> + regs.SYSCALL_NUM = syscall;
> + }
>
> #elif defined(__arm__)
> # ifndef PTRACE_SET_SYSCALL
> @@ -1327,7 +1344,11 @@ void change_syscall(struct __test_metadata *_metadata,
>
> /* If syscall is skipped, change return value. */
> if (syscall == -1)
> +#ifdef SYSCALL_NUM_RET_SHARE_REG
> + TH_LOG("Can't modify syscall return on this architecture");
> +#else
> regs.SYSCALL_RET = 1;
> +#endif
>
> #ifdef HAVE_GETREGS
> ret = ptrace(PTRACE_SETREGS, tracee, 0, &regs);
> @@ -1465,8 +1486,13 @@ TEST_F(TRACE_syscall, syscall_dropped)
> ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
> ASSERT_EQ(0, ret);
>
> +#ifdef SYSCALL_NUM_RET_SHARE_REG
> + /* gettid has been skipped */
> + EXPECT_EQ(-1, syscall(__NR_gettid));
> +#else
> /* gettid has been skipped and an altered return value stored. */
> EXPECT_EQ(1, syscall(__NR_gettid));
> +#endif
> EXPECT_NE(self->mytid, syscall(__NR_gettid));
> }
>
> --
> 2.5.0
>

Acked-by: Kees Cook <[email protected]>

Thanks!

-Kees

--
Kees Cook
Chrome OS & Brillo Security