[PATCH v2 0/4] syscall_trace fixes and cleanups

[PATCH v2 0/4] syscall_trace fixes and cleanups

[Will Deacon]

Hello,

This is version 2 of the patches originally posted here:

  http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/097748.html

The only changes are that (a) two of the original patches (fixes) have
been now been merged into mainline and (b) the remaining patches in the
series are now rebased onto -rc1, which includes Al's work on the signal
handling / syscall restarting path.

All comments welcome,

Will


Will Deacon (4):
  audit: arm: only allow syscall auditing for pure EABI userspace
  ARM: entry: don't bother with syscall tracing on ret_from_fork path
  ARM: audit: move syscall auditing until after ptrace SIGTRAP handling
  ARM: ptrace: provide separate functions for tracing syscall
    {entry,exit}

 arch/arm/kernel/entry-common.S |   20 +++++-------------
 arch/arm/kernel/ptrace.c       |   42 ++++++++++++++++++++++++++++-----------
 init/Kconfig                   |    2 +-
 3 files changed, 37 insertions(+), 27 deletions(-)

-- 
1.7.4.1

[PATCH v2 1/4] audit: arm: only allow syscall auditing for pure EABI userspace

[Will Deacon]

The audit tools support only EABI userspace and, since there are no
AUDIT_ARCH_* defines for the ARM OABI, it makes sense to allow syscall
auditing on ARM only for EABI at the moment.

Cc: Eric Paris <eparis@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 init/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index d07dcf9..b3f55f1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -357,7 +357,7 @@ config AUDIT
 
 config AUDITSYSCALL
 	bool "Enable system-call auditing support"
-	depends on AUDIT && (X86 || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || ARM)
+	depends on AUDIT && (X86 || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
 	default y if SECURITY_SELINUX
 	help
 	  Enable low-overhead system-call auditing infrastructure that
-- 
1.7.4.1

[PATCH v2 2/4] ARM: entry: don't bother with syscall tracing on ret_from_fork path

[Will Deacon]

ret_from_fork is setup for a freshly spawned child task via copy_thread,
called from copy_process. The latter function clears TIF_SYSCALL_TRACE
and also resets the child task's audit_context to NULL, meaning that
there is little point invoking the system call tracing routines.
Furthermore, getting hold of the syscall number is a complete pain and
it looks like the current code doesn't even bother.

This patch removes the syscall tracing checks from ret_from_fork.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/entry-common.S |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 4afed88..10911c9 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -95,13 +95,7 @@ ENDPROC(ret_to_user)
 ENTRY(ret_from_fork)
 	bl	schedule_tail
 	get_thread_info tsk
-	ldr	r1, [tsk, #TI_FLAGS]		@ check for syscall tracing
 	mov	why, #1
-	tst	r1, #_TIF_SYSCALL_WORK		@ are we tracing syscalls?
-	beq	ret_slow_syscall
-	mov	r1, sp
-	mov	r0, #1				@ trace exit [IP = 1]
-	bl	syscall_trace
 	b	ret_slow_syscall
 ENDPROC(ret_from_fork)
 
-- 
1.7.4.1

[PATCH v2 3/4] ARM: audit: move syscall auditing until after ptrace SIGTRAP handling

[Will Deacon]

When auditing system calls on ARM, the audit code is called before
notifying the parent process in the case that the current task is being
ptraced. At this point, the parent (debugger) may choose to change the
system call being issued via the SET_SYSCALL ptrace request, causing
the wrong system call to be reported to the audit tools.

This patch moves the audit calls after the ptrace SIGTRAP handling code
in the syscall tracing implementation.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/ptrace.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 5700a7a..5256068 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -912,16 +912,11 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno)
 {
 	unsigned long ip;
 
-	if (why)
-		audit_syscall_exit(regs);
-	else
-		audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0,
-				    regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
-
 	if (why == 0 && test_and_clear_thread_flag(TIF_SYSCALL_RESTARTSYS))
 		scno = __NR_restart_syscall - __NR_SYSCALL_BASE;
+
 	if (!test_thread_flag(TIF_SYSCALL_TRACE))
-		return scno;
+		goto out_no_trace;
 
 	current_thread_info()->syscall = scno;
 
@@ -938,6 +933,13 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno)
 		current_thread_info()->syscall = -1;
 
 	regs->ARM_ip = ip;
+	scno = current_thread_info()->syscall;
 
-	return current_thread_info()->syscall;
+out_no_trace:
+	if (why)
+		audit_syscall_exit(regs);
+	else
+		audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0,
+				    regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
+	return scno;
 }
-- 
1.7.4.1

[PATCH v2 4/4] ARM: ptrace: provide separate functions for tracing syscall {entry, exit}

[Will Deacon]

The syscall_trace on ARM takes a `why' parameter to indicate whether or
not we are entering or exiting a system call. This can be confusing for
people looking at the code since (a) it conflicts with the why register
alias in the entry assembly code and (b) it is not immediately clear
what it represents.

This patch splits up the syscall_trace function into separate wrappers
for syscall entry and exit, allowing the low-level syscall handling
code to branch to the appropriate function.

Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/entry-common.S |   14 +++++-------
 arch/arm/kernel/ptrace.c       |   46 +++++++++++++++++++++++++++-------------
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 10911c9..49d9f93 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -442,10 +442,9 @@ ENDPROC(vector_swi)
 	 * context switches, and waiting for our parent to respond.
 	 */
 __sys_trace:
-	mov	r2, scno
-	add	r1, sp, #S_OFF
-	mov	r0, #0				@ trace entry [IP = 0]
-	bl	syscall_trace
+	mov	r1, scno
+	add	r0, sp, #S_OFF
+	bl	syscall_trace_enter
 
 	adr	lr, BSYM(__sys_trace_return)	@ return address
 	mov	scno, r0			@ syscall number (possibly new)
@@ -457,10 +456,9 @@ __sys_trace:
 
 __sys_trace_return:
 	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
-	mov	r2, scno
-	mov	r1, sp
-	mov	r0, #1				@ trace exit [IP = 1]
-	bl	syscall_trace
+	mov	r1, scno
+	mov	r0, sp
+	bl	syscall_trace_exit
 	b	ret_slow_syscall
 
 	.align	5
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 5256068..749899d 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -908,15 +908,18 @@ long arch_ptrace(struct task_struct *child, long request,
 	return ret;
 }
 
-asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno)
+enum ptrace_syscall_dir {
+	PTRACE_SYSCALL_ENTER = 0,
+	PTRACE_SYSCALL_EXIT,
+};
+
+static int ptrace_syscall_trace(struct pt_regs *regs, int scno,
+				enum ptrace_syscall_dir dir)
 {
 	unsigned long ip;
 
-	if (why == 0 && test_and_clear_thread_flag(TIF_SYSCALL_RESTARTSYS))
-		scno = __NR_restart_syscall - __NR_SYSCALL_BASE;
-
 	if (!test_thread_flag(TIF_SYSCALL_TRACE))
-		goto out_no_trace;
+		return scno;
 
 	current_thread_info()->syscall = scno;
 
@@ -925,21 +928,34 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int scno)
 	 * IP = 0 -> entry, =1 -> exit
 	 */
 	ip = regs->ARM_ip;
-	regs->ARM_ip = why;
+	regs->ARM_ip = dir;
 
-	if (why)
+	if (dir == PTRACE_SYSCALL_EXIT)
 		tracehook_report_syscall_exit(regs, 0);
 	else if (tracehook_report_syscall_entry(regs))
 		current_thread_info()->syscall = -1;
 
 	regs->ARM_ip = ip;
-	scno = current_thread_info()->syscall;
+	return current_thread_info()->syscall;
+}
 
-out_no_trace:
-	if (why)
-		audit_syscall_exit(regs);
-	else
-		audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0,
-				    regs->ARM_r1, regs->ARM_r2, regs->ARM_r3);
-	return scno;
+asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
+{
+	int ret;
+
+	if (test_and_clear_thread_flag(TIF_SYSCALL_RESTARTSYS))
+		scno = __NR_restart_syscall - __NR_SYSCALL_BASE;
+
+	ret = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_ENTER);
+	audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs->ARM_r0, regs->ARM_r1,
+			    regs->ARM_r2, regs->ARM_r3);
+
+	return ret;
+}
+
+asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
+{
+	int ret = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
+	audit_syscall_exit(regs);
+	return ret;
 }
-- 
1.7.4.1

[PATCH v2 0/4] syscall_trace fixes and cleanups

[Russell King - ARM Linux]

On Fri, Jun 08, 2012 at 04:12:17PM +0100, Will Deacon wrote:
> Hello,
> 
> This is version 2 of the patches originally posted here:
> 
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/097748.html
> 
> The only changes are that (a) two of the original patches (fixes) have
> been now been merged into mainline and (b) the remaining patches in the
> series are now rebased onto -rc1, which includes Al's work on the signal
> handling / syscall restarting path.
> 
> All comments welcome,

It may be a good idea to Cc Al on these.

[PATCH v2 0/4] syscall_trace fixes and cleanups

[Will Deacon]

Hi Russell,

On Sat, Jun 09, 2012 at 01:38:34PM +0100, Russell King - ARM Linux wrote:
> On Fri, Jun 08, 2012 at 04:12:17PM +0100, Will Deacon wrote:
> > The only changes are that (a) two of the original patches (fixes) have
> > been now been merged into mainline and (b) the remaining patches in the
> > series are now rebased onto -rc1, which includes Al's work on the signal
> > handling / syscall restarting path.
> > 
> > All comments welcome,
> 
> It may be a good idea to Cc Al on these.

Sure. Since I've not received any other comments on this series, I'll
repost this week with Al on CC.

Will