| Message ID | 20200912110820.597135-4-keescook@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/seccomp: Refactor change_syscall() | expand |
On Sat, Sep 12, 2020 at 04:08:08AM -0700, Kees Cook wrote: > Remove the mips special-case in change_syscall(). > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index 1c83e743bfb1..02a9a6599746 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -1742,6 +1742,13 @@ TEST_F(TRACE_poke, getpid_runs_normally) > # define ARCH_REGS struct pt_regs > # define SYSCALL_NUM(_regs) (_regs).regs[2] > # define SYSCALL_SYSCALL_NUM regs[4] > +# define SYSCALL_NUM_SET(_regs, _nr) \ > + do { \ > + if ((_regs).regs[2] == __NR_O32_Linux) \ > + (_regs).regs[4] = _nr; \ > + else \ > + (_regs).regs[2] = _nr; \ > + } while (0) I think that # define SYSCALL_NUM_SET(_regs, _nr) \ do { \ if (SYSCALL_NUM(_regs) == __NR_O32_Linux) \ (_regs).regs[4] = _nr; \ else \ (_regs).regs[2] = _nr; \ } while (0) would read better but that's just a matter of taste. :) Looks good! Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
On Tue, Sep 15, 2020 at 05:55:46PM +0200, Christian Brauner wrote: > On Sat, Sep 12, 2020 at 04:08:08AM -0700, Kees Cook wrote: > > Remove the mips special-case in change_syscall(). > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > tools/testing/selftests/seccomp/seccomp_bpf.c | 17 +++++++++-------- > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > > index 1c83e743bfb1..02a9a6599746 100644 > > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > > @@ -1742,6 +1742,13 @@ TEST_F(TRACE_poke, getpid_runs_normally) > > # define ARCH_REGS struct pt_regs > > # define SYSCALL_NUM(_regs) (_regs).regs[2] > > # define SYSCALL_SYSCALL_NUM regs[4] > > +# define SYSCALL_NUM_SET(_regs, _nr) \ > > + do { \ > > + if ((_regs).regs[2] == __NR_O32_Linux) \ > > + (_regs).regs[4] = _nr; \ > > + else \ > > + (_regs).regs[2] = _nr; \ > > + } while (0) > > I think that > > # define SYSCALL_NUM_SET(_regs, _nr) \ > do { \ > if (SYSCALL_NUM(_regs) == __NR_O32_Linux) \ > (_regs).regs[4] = _nr; \ > else \ > (_regs).regs[2] = _nr; \ > } while (0) > > would read better but that's just a matter of taste. :) That's how I started originally, but when I realized that I'd have to reorganize SYSCALL_NUM() too, it seem best to have minimal churn, so I left it open coded here, since that's how it needs to be in the end. > Looks good! > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Thanks for the reviews!
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 1c83e743bfb1..02a9a6599746 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -1742,6 +1742,13 @@ TEST_F(TRACE_poke, getpid_runs_normally) # define ARCH_REGS struct pt_regs # define SYSCALL_NUM(_regs) (_regs).regs[2] # define SYSCALL_SYSCALL_NUM regs[4] +# define SYSCALL_NUM_SET(_regs, _nr) \ + do { \ + if ((_regs).regs[2] == __NR_O32_Linux) \ + (_regs).regs[4] = _nr; \ + else \ + (_regs).regs[2] = _nr; \ + } while (0) # define SYSCALL_RET(_regs) (_regs).regs[2] # define SYSCALL_NUM_RET_SHARE_REG #elif defined(__xtensa__) @@ -1839,17 +1846,11 @@ void change_syscall(struct __test_metadata *_metadata, #if defined(__x86_64__) || defined(__i386__) || defined(__powerpc__) || \ defined(__s390__) || defined(__hppa__) || defined(__riscv) || \ - defined(__xtensa__) || defined(__csky__) || defined(__sh__) + defined(__xtensa__) || defined(__csky__) || defined(__sh__) || \ + defined(__mips__) { SYSCALL_NUM_SET(regs, syscall); } -#elif defined(__mips__) - { - if (SYSCALL_NUM(regs) == __NR_O32_Linux) - regs.SYSCALL_SYSCALL_NUM = syscall; - else - SYSCALL_NUM_SET(regs, syscall); - } #elif defined(__arm__) # ifndef PTRACE_SET_SYSCALL
Remove the mips special-case in change_syscall(). Signed-off-by: Kees Cook <keescook@chromium.org> --- tools/testing/selftests/seccomp/seccomp_bpf.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)