Re: [PATCH] selftests/seccomp: fix ptrace tests on powerpc

[Kees Cook <keescook@chromium.org>]

On Sun, Sep 13, 2020 at 10:34:23PM +1000, Michael Ellerman wrote:
> Thadeu Lima de Souza Cascardo <cascardo@canonical.com> 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