linux-kernel - Re: [PATCH] selftests/seccomp: add MIPS self-test support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGXu5jLexb=4ySrnz90xRdrK5X3KO0N_hy0gd1Lieaw3p64qqQ@mail.gmail.com>
Date:	Mon, 21 Mar 2016 11:45:29 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Matt Redfearn <matt.redfearn@...tec.com>,
	Shuah Khan <shuahkh@....samsung.com>
Cc:	milko.leporis@...tec.com, Ralf Baechle <ralf@...ux-mips.org>,
	Andy Lutomirski <luto@...capital.net>,
	Will Drewry <wad@...omium.org>,
	linux-kselftest@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Linux MIPS Mailing List <linux-mips@...ux-mips.org>
Subject: Re: [PATCH] selftests/seccomp: add MIPS self-test support

On Thu, Mar 17, 2016 at 2:26 AM, Matt Redfearn <matt.redfearn@...tec.com> 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 <matt.redfearn@...tec.com>
> ---
>  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 <keescook@...omium.org>

Thanks!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security