Messages in this thread | | | Date | Sat, 4 Jul 2020 13:50:27 +0100 | From | Will Deacon <> | Subject | Re: ptrace: seccomp: Return value when the call was already invalid |
| |
On Fri, Jul 03, 2020 at 04:27:37PM -0400, Keno Fischer wrote: > > > Now, if we have a seccomp filter that simply does > > > SECCOMP_RET_TRACE, and a ptracer that simply > > > does PTRACE_CONT > > > > Ok, so this means that we're _skipping_ the system call, right? > > If the system call were positive this would result in the system call > being executed. The notion of "skipping" the syscall is a bit odd in > this situation. Having the ptracer set the syscallno to -1 is generally > accepted as the way to do it, but what happens if the syscallno is > already -1 or negative is underspecified.
Ok. I think it would be sensible for us to have the same behaviour for all negative system calls though.
> > > then the assert will fire/fail on arm64, but not on x86_64. > > > > It feels weird to me that skipping the system call has any effect on the > > tracee registers... > > I think the correct way to frame it is to ask whether the behavior > matches that of the tracee in absence of the ptracer. I would argue > that if the ptracer doesn't explicitly modify register contents, then > the tracee shouldn't observe any behavior difference.
That's a useful way of thinking about it and is still the case after this patch. The difference now is that x0 isn't zapped in the case where a syscall(-1) is skipped.
> > > Interestingly, arm64 does do something different > > > if the syscall is -1 rather than -10, where early > > > in the ptrace stop it does. > > > ``` > > > /* set default errno for user-issued syscall(-1) */ > > > if (scno == NO_SYSCALL) > > > regs->regs[0] = -ENOSYS; > > > > ... so I think this should be fixed too. How about the diff below? > > I think the patch behavior is better overall, but I'm not sure it's ideal. > I think the biggest question is what the behavior should be here and > if we want a behavioral difference between *the syscall was -1 at entry* > and *the syscall was -1 because the ptracer wanted to skip the syscall*. > I think there is a bit of a semantic disconnect because "skipping" the > syscall is not really an operation that the ptracer has at its disposal > (unless it's using SYSEMU of course). The only thing it can do is set > the syscall to -1. However, arguably that already has semantics > (of returning -ENOSYS), so it's not at all clear to me that we should > deviate from that. Unfortunately, none of this is currently consistent > across architectures, so I think before we go changing arm64, we > should decide what we'd like to happen in theory and then see > what we can do to improve the situation without being too breaking.
I just object to treating a user-issued -1 differently to any other negative system call. With this patch, they're all treated the same, which is to say that they return -ENOSYS normally, but when skipped by a tracer (e.g. by setting the syscall number to -1 or because of a seccomp failure), then x0 is preserved.
This means that, with this patch, skipping syscall(-1) behaves the same way as skipping syscall(-2) with mainline today.
Will
| |