Subject: [PATCH] arm64: fix missing syscall trace exit

From: [email-masked] (Will Deacon)
Date: Wed, 3 Jun 2015 10:52:41 +0100

On Wed, Jun 03, 2015 at 02:11:48AM +0100, Josh Stone wrote:
> On 06/02/2015 06:01 PM, Josh Stone wrote:
> > If a syscall is entered without TIF_SYSCALL_TRACE set, then it goes on
> > the fast path.  It's then possible to have TIF_SYSCALL_TRACE added in
> > the middle of the syscall, but ret_fast_syscall doesn't check this flag
> > again.  This causes a ptrace syscall-exit-stop to be missed.
> > 
> > For instance, from a PTRACE_EVENT_FORK reported during do_fork, the
> > tracer might resume with PTRACE_SYSCALL, setting TIF_SYSCALL_TRACE.
> > Now the completion of the fork should have a syscall-exit-stop.
> > 
> > Russell King fixed this on arm by re-checking _TIF_SYSCALL_WORK in the
> > fast exit path.  Do the same on arm64.
> > 
> > Cc: Catalin Marinas <[email-masked]>
> > Cc: Will Deacon <[email-masked]>
> > Cc: Russell King <[email-masked]>
> > Signed-off-by: Josh Stone <[email-masked]>
> > ---
> >  arch/arm64/kernel/entry.S | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index 959fe8733560..a547a3e8a198 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -608,7 +608,9 @@ ENDPROC(cpu_switch_to)
> >   */
> >  ret_fast_syscall:
> >  	disable_irq				// disable interrupts
> > -	ldr	x1, [tsk, #TI_FLAGS]
> > +	ldr	x1, [tsk, #TI_FLAGS]		// re-check for syscall tracing
> > +	and	x2, x1, #_TIF_SYSCALL_WORK
> > +	cbnz	x2, __sys_trace_return
> >  	and	x2, x1, #_TIF_WORK_MASK
> >  	cbnz	x2, fast_work_pending
> >  	enable_step_tsk x1, x2
> 
> I do have one concern about this, also in Russell's ARM patch.  Is it
> really ok to branch to __sys_trace_return with interrupts disabled?

I think you're right to be concerned!

> I didn't hit any issue from that, but my testcase only exercises this
> path once each run.  So that might have just been lucky not to hit any
> gross scenario...

Did you try enabling all the audit stuff? It looks like that can call
into the scheduler, so I think we should be running the tracing callbacks
with interrupts enabled (and it looks like x86 do this on the exit path).

Will