On Sun, Sep 13, 2020 at 10:34:23PM +1000, Michael Ellerman wrote: > Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxx> writes: > > On Tue, Sep 08, 2020 at 04:18:17PM -0700, Kees Cook wrote: > >> On Tue, Jun 30, 2020 at 01:47:39PM -0300, Thadeu Lima de Souza Cascardo wrote: > ... > >> > @@ -1809,10 +1818,15 @@ void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee, > >> > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > >> > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); > >> > > >> > - if (!entry) > >> > + if (!entry && !syscall_nr) > >> > return; > >> > > >> > - nr = get_syscall(_metadata, tracee); > >> > + if (entry) > >> > + nr = get_syscall(_metadata, tracee); > >> > + else > >> > + nr = *syscall_nr; > >> > >> This is weird? Shouldn't get_syscall() be modified to do the right thing > >> here instead of depending on the extra arg? > >> > > > > R0 might be clobered. Same documentation mentions it as volatile. So, during > > syscall exit, we can't tell for sure that R0 will have the original syscall > > number. So, we need to grab it during syscall enter, save it somewhere and > > reuse it. I used the test context/args for that. > > The user r0 (in regs->gpr[0]) shouldn't be clobbered. > > But it is modified if the tracer skips the syscall, by setting the > syscall number to -1. Or if the tracer changes the syscall number. > > So if you need the original syscall number in the exit path then I think > you do need to save it at entry. ... the selftest code wants to test the updated syscall (-1 or whatever), so this sounds correct. Was this test actually failing on powerpc? (I'd really rather not split entry/exit if I don't have to.) -- Kees Cook
