Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

[Richard Guy Briggs <rgb@redhat.com>]

On 14/03/15, AKASHI Takahiro wrote:
> As done in arm, this change makes it easy to confirm we invoke syscall
> related hooks, including syscall tracepoint, audit and seccomp which would
> be implemented later, in correct order. That is, undoing operations in the
> opposite order on exit that they were done on entry.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Minor variable mis-spelling of "scratch" noted below, but other than
that:

Acked-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  arch/arm64/kernel/entry.S  | 10 ++++------
>  arch/arm64/kernel/ptrace.c | 50 +++++++++++++++++++++++++++-------------------
>  2 files changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index f9f2cae..00d6eb9 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -649,9 +649,8 @@ ENDPROC(el0_svc)
>  	 * switches, and waiting for our parent to respond.
>  	 */
>  __sys_trace:
> -	mov	x1, sp
> -	mov	w0, #0				// trace entry
> -	bl	syscall_trace
> +	mov	x0, sp
> +	bl	syscall_trace_enter
>  	adr	lr, __sys_trace_return		// return address
>  	uxtw	scno, w0			// syscall number (possibly new)
>  	mov	x1, sp				// pointer to regs
> @@ -666,9 +665,8 @@ __sys_trace:
>  
>  __sys_trace_return:
>  	str	x0, [sp]			// save returned x0
> -	mov	x1, sp
> -	mov	w0, #1				// trace exit
> -	bl	syscall_trace
> +	mov	x0, sp
> +	bl	syscall_trace_exit
>  	b	ret_to_user
>  
>  /*
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6a8928b..f606276 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1058,35 +1058,43 @@ long arch_ptrace(struct task_struct *child, long request,
>  	return ptrace_request(child, request, addr, data);
>  }
>  
> -asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> +enum ptrace_syscall_dir {
> +	PTRACE_SYSCALL_ENTER = 0,
> +	PTRACE_SYSCALL_EXIT,
> +};
> +
> +static void tracehook_report_syscall(struct pt_regs *regs,
> +				     enum ptrace_syscall_dir dir)
>  {
> +	int scrach;

"scratch"

>  	unsigned long saved_reg;
>  
> -	if (!test_thread_flag(TIF_SYSCALL_TRACE))
> -		return regs->syscallno;
> -
> -	if (is_compat_task()) {
> -		/* AArch32 uses ip (r12) for scratch */
> -		saved_reg = regs->regs[12];
> -		regs->regs[12] = dir;
> -	} else {
> -		/*
> -		 * Save X7. X7 is used to denote syscall entry/exit:
> -		 *   X7 = 0 -> entry, = 1 -> exit
> -		 */
> -		saved_reg = regs->regs[7];
> -		regs->regs[7] = dir;
> -	}
> +	/*
> +	 * A scrach register (ip(r12) on AArch32, x7 on AArch64) is
> +	 * used to denote syscall entry/exit:
> +	 */
> +	scrach = (is_compat_task() ? 12 : 7);
> +	saved_reg = regs->regs[scrach];
> +	regs->regs[scrach] = dir;
>  
> -	if (dir)
> +	if (dir == PTRACE_SYSCALL_EXIT)
>  		tracehook_report_syscall_exit(regs, 0);
>  	else if (tracehook_report_syscall_entry(regs))
>  		regs->syscallno = ~0UL;
>  
> -	if (is_compat_task())
> -		regs->regs[12] = saved_reg;
> -	else
> -		regs->regs[7] = saved_reg;
> +	regs->regs[scrach] = saved_reg;
> +}
> +
> +asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> +{
> +	if (test_thread_flag(TIF_SYSCALL_TRACE))
> +		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
>  
>  	return regs->syscallno;
>  }
> +
> +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
> +{
> +	if (test_thread_flag(TIF_SYSCALL_TRACE))
> +		tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
> +}
> -- 
> 1.8.3.2

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545