Kees Cook <keesc...@chromium.org> writes: > Some archs (like ppc) only support changing the return code during > syscall exit when ptrace is used. As the syscall number might not > be available anymore during syscall exit, it needs to be saved > during syscall enter. Adjust the ptrace tests to do this.
I'm not that across all the fixture stuff, but if I'm reading it right you're now calling change_syscall() on both entry and exit for all arches. That should work, but it no longer tests changing the return code on entry on the arches that support it, which seems like a backward step? cheers > Reported-by: Thadeu Lima de Souza Cascardo <casca...@canonical.com> > Suggested-by: Thadeu Lima de Souza Cascardo <casca...@canonical.com> > Link: > https://lore.kernel.org/linux-kselftest/20200911181012.171027-1-casca...@canonical.com/ > Fixes: 58d0a862f573 ("seccomp: add tests for ptrace hole") > Signed-off-by: Kees Cook <keesc...@chromium.org> > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 34 +++++++++++-------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c > b/tools/testing/selftests/seccomp/seccomp_bpf.c > index bbab2420d708..26c712c6a575 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -1949,12 +1949,19 @@ void tracer_seccomp(struct __test_metadata > *_metadata, pid_t tracee, > > } > > +FIXTURE(TRACE_syscall) { > + struct sock_fprog prog; > + pid_t tracer, mytid, mypid, parent; > + long syscall_nr; > +}; > + > void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee, > int status, void *args) > { > - int ret, nr; > + int ret; > unsigned long msg; > static bool entry; > + FIXTURE_DATA(TRACE_syscall) *self = args; > > /* > * The traditional way to tell PTRACE_SYSCALL entry/exit > @@ -1968,24 +1975,23 @@ void tracer_ptrace(struct __test_metadata *_metadata, > pid_t tracee, > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); > > - if (!entry) > - return; > - > - nr = get_syscall(_metadata, tracee); > + /* > + * Some architectures only support setting return values during > + * syscall exit under ptrace, and on exit the syscall number may > + * no longer be available. Therefore, save it here, and call > + * "change syscall and set return values" on both entry and exit. > + */ > + if (entry) > + self->syscall_nr = get_syscall(_metadata, tracee); > > - if (nr == __NR_getpid) > + if (self->syscall_nr == __NR_getpid) > change_syscall(_metadata, tracee, __NR_getppid, 0); > - if (nr == __NR_gettid) > + if (self->syscall_nr == __NR_gettid) > change_syscall(_metadata, tracee, -1, 45000); > - if (nr == __NR_openat) > + if (self->syscall_nr == __NR_openat) > change_syscall(_metadata, tracee, -1, -ESRCH); > } > > -FIXTURE(TRACE_syscall) { > - struct sock_fprog prog; > - pid_t tracer, mytid, mypid, parent; > -}; > - > FIXTURE_VARIANT(TRACE_syscall) { > /* > * All of the SECCOMP_RET_TRACE behaviors can be tested with either > @@ -2044,7 +2050,7 @@ FIXTURE_SETUP(TRACE_syscall) > self->tracer = setup_trace_fixture(_metadata, > variant->use_ptrace ? tracer_ptrace > : tracer_seccomp, > - NULL, variant->use_ptrace); > + self, variant->use_ptrace); > > ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > -- > 2.25.1
