ARM: 7595/1: syscall: rework ordering in syscall_trace_exit

syscall_trace_exit is currently doing things back-to-front; invoking
the audit hook *after* signalling the debugger, which presents an
opportunity for the registers to be re-written by userspace in order to
bypass auditing constaints.

This patch fixes the ordering by moving the audit code first and the
tracehook code last. On the face of it, it looks like
current_thread_info()->syscall may be incorrect for the sys_exit
tracepoint, but that's actually not an issue because it will have been
set during syscall entry and cannot have changed since then.

Reported-by: Andrew Gabbasov <Andrew_Gabbasov@mentor.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index ee81dbc6fa1028c912ce927316eaa3fd4d584061..d863bbf0f1f507555eaaf3a995e405f9c946a438 100644 (file)
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -455,7 +455,6 @@ __sys_trace:
 
 __sys_trace_return:
        str     r0, [sp, #S_R0 + S_OFF]!        @ save returned r0
-       mov     r1, scno
        mov     r0, sp
        bl      syscall_trace_exit
        b       ret_slow_syscall

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 518536d93fba415e301d932b4e5fe2592a262ff8..03deeffd9f6d06e6ff380126592e10dbf7bf1a25 100644 (file)
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -957,17 +957,23 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
        return scno;
 }
 
-asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
+asmlinkage void syscall_trace_exit(struct pt_regs *regs)
 {
-       current_thread_info()->syscall = scno;
-
-       if (test_thread_flag(TIF_SYSCALL_TRACE))
-               scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
+       /*
+        * Audit the syscall before anything else, as a debugger may
+        * come in and change the current registers.
+        */
+       audit_syscall_exit(regs);
 
+       /*
+        * Note that we haven't updated the ->syscall field for the
+        * current thread. This isn't a problem because it will have
+        * been set on syscall entry and there hasn't been an opportunity
+        * for a PTRACE_SET_SYSCALL since then.
+        */
        if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
-               trace_sys_exit(regs, scno);
-
-       audit_syscall_exit(regs);
+               trace_sys_exit(regs, regs_return_value(regs));
 
-       return scno;
+       if (test_thread_flag(TIF_SYSCALL_TRACE))
+               tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
 }