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

[Michael Ellerman <mpe@ellerman.id.au>]

Thadeu Lima de Souza Cascardo <cascardo@canonical.com> writes:
> On Thu, Sep 17, 2020 at 03:37:16PM -0700, Kees Cook wrote:
>> 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.)
>
> Yes, it started failing when the return code started being changed as well.
> Though ptrace can change the syscall at entry (IIRC), it can't change the
> return code. And that is part of the ABI. If ppc is changed so it allows
> changing the return code during ptrace entry, some strace uses will break. So
> that is not an option.

Yep.

I don't know that it would break anything to change that part of the
ptrace ABI, but it definitely could break things and it would be subtle,
so it's not really an option.

So for powerpc we do need the return code change done in the exit hook,
sorry.

cheers