On Wed, Jun 3, 2020 at 11:53 AM Will Deacon <will@xxxxxxxxxx> wrote:
> > However, at the same time as changing this, we should probably make sure
> > to enable the syscall exit pseudo-singlestep trap (similar issue as the other
> > patch I had sent for the signal pseudo-singlestep trap), since otherwise
> > ptracers might get confused about the lack of singlestep trap during a
> > singlestep -> seccomp -> singlestep path (which would give one trap
> > less with this patch than before).
>
> Hmm, I don't completely follow your example. Please could you spell it
> out a bit more? I fast-forward the stepping state machine on sigreturn,
> which I thought would be sufficient. Perhaps you're referring to a variant
> of the situation mentioned by Mark, which I didn't think could happen
> with ptrace [2].
Sure suppose we have code like the following:
0x0: svc #0
0x4: str x0, [x7]
...
Then, if there's a seccomp filter active that just does
SECCOMP_RET_TRACE of everything, right now we get traps:
<- (ip: 0x0)
-> PTRACE_SINGLESTEP
<- (ip: 0x4 - seccomp trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x4 - TRAP_TRACE trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
With your proposed patch, we instead get
<- (ip: 0x0)
-> PTRACE_SINGLESTEP
<- (ip: 0x4 - seccomp trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
This is problematic, because the ptracer may want to inspect the
result of the syscall instruction. On other architectures, this
problem is solved with a pseudo-singlestep trap that gets executed
if you resume from a syscall-entry-like trap with PTRACE_SINGLESTEP.
See the below patch for the change I'm proposing. There is a slight
issue with that patch, still: It now makes the x7 issue apply to the
singlestep trap at exit, so we should do the patch to fix that issue
before we apply that change (or manually check for this situation
and issue the pseudo-singlestep trap manually).
My proposed patch below also changes
<- (ip: 0x0)
-> PTRACE_SYSCALL
<- (ip: 0x4 - syscall entry trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
to
<- (ip: 0x0)
-> PTRACE_SYSCALL
<- (ip: 0x4 - syscall entry trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x4 - pseudo-singlestep exit trap)
-> PTRACE_SINGLESTEP
<- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
But I consider that a bugfix, since that's how other architectures
behave and I was going to send in this patch for that reason anyway
(since this was another one of the aarch64 ptrace quirks we had to
work around).
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b3d3005d9515..104cfcf117d0 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1820,7 +1820,7 @@ static void tracehook_report_syscall(struct pt_regs *regs,
regs->regs[regno] = dir;
if (dir == PTRACE_SYSCALL_EXIT)
- tracehook_report_syscall_exit(regs, 0);
+ tracehook_report_syscall_exit(regs,
test_thread_flag(TIF_SINGLESTEP));
else if (tracehook_report_syscall_entry(regs))
forget_syscall(regs);
