audit: Syscall rules are not applied to existing processes on non-x86

PATCH In review Anton Blanchard 2 participants 6 messages Jan 8, 2013 – Apr 10, 2013

Subsystems (3)

PowerPC ptrace support Audit

lore

Share & tools

lore

Anton Blanchard

author

2013-01-08 23:46 UTC #0 lore raw

PATCH 1/4 audit: Syscall rules are not applied to existing processes on non-x86

Commit b05d8447e782 (audit: inline audit_syscall_entry to reduce burden on archs) changed audit_syscall_entry to check for a dummy context before calling __audit_syscall_entry. Unfortunately the dummy context state is maintained in __audit_syscall_entry so once set it never gets cleared, even if the audit rules change.

As a result, if there are no auditing rules when a process starts then it will never be subject to any rules added later. x86 doesn't see this because it has an assembly fast path that calls directly into __audit_syscall_entry.

I noticed this issue when working on audit performance optimisations. I wrote a set of simple test cases available at:

http://ozlabs.org/~anton/junkcode/audit_tests.tar.gz

02_new_rule.py fails without the patch and passes with it. The test case clears all rules, starts a process, adds a rule then verifies the process produces a syscall audit record.

Signed-off-by Anton Blanchard <[email protected]>

Cc <[email protected]> # 3.3+

---

include/linux/audit.h +1 -1

Index: b/include/linux/audit.h
===================================================================
@@ -119,7 +119,7 @@ static inline void audit_syscall_entry(i
 				       unsigned long a1, unsigned long a2,
 				       unsigned long a3)
 {
-	if (unlikely(!audit_dummy_context()))
+	if (unlikely(current->audit_context))
 		__audit_syscall_entry(arch, major, a0, a1, a2, a3);
 }
 static inline void audit_syscall_exit(void *pt_regs)

Anton Blanchard

author

2013-01-08 23:47 UTC #1 lore raw

PATCH 2/4 powerpc: Remove static branch prediction in 64bit traced syscall path

Some distros enable auditing by default which forces us through the syscall trace path. Remove the static branch prediction in our 64bit syscall handler and let the hardware do the prediction.

Signed-off-by Anton Blanchard <[email protected]>

---

arch/powerpc/kernel/entry_64.S +1 -1

Index: b/arch/powerpc/kernel/entry_64.S
===================================================================
@@ -149,7 +149,7 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLP
 	CURRENT_THREAD_INFO(r11, r1)
 	ld	r10,TI_FLAGS(r11)
 	andi.	r11,r10,_TIF_SYSCALL_T_OR_A
-	bne-	syscall_dotrace
+	bne	syscall_dotrace
 .Lsyscall_dotrace_cont:
 	cmpldi	0,r0,NR_syscalls
 	bge-	syscall_enosys

Anton Blanchard

author

2013-01-08 23:48 UTC #2 lore raw

PATCH 3/4 powerpc: Optimise 64bit syscall auditing entry path

In reply to Anton Blanchard

Add an assembly fast path for the syscall audit entry path on 64bit. Some distros enable auditing by default which forces us through the syscall auditing path even if there are no rules.

I wrote some test cases to validate the patch:

http://ozlabs.org/~anton/junkcode/audit_tests.tar.gz

And to test the performance I ran a simple null syscall microbenchmark on a POWER7 box:

http://ozlabs.org/~anton/junkcode/null_syscall.c

Baseline: 949.2 cycles
Patched:  920.6 cycles

An improvement of 3%. Most of the potential gains are masked by the syscall audit exit path which will be fixed in a subsequent patch.

Signed-off-by Anton Blanchard <[email protected]>

---

arch/powerpc/kernel/entry_64.S +39 -0

Index: b/arch/powerpc/kernel/entry_64.S
===================================================================
@@ -34,6 +34,12 @@
 #include <asm/ftrace.h>
 #include <asm/hw_irq.h>
 
+/* Avoid __ASSEMBLER__'ifying <linux/audit.h> just for this.  */
+#include <linux/elf-em.h>
+#define AUDIT_ARCH_PPC		(EM_PPC)
+#define AUDIT_ARCH_PPC64	(EM_PPC64|__AUDIT_ARCH_64BIT)
+#define __AUDIT_ARCH_64BIT 0x80000000
+
 /*
  * System calls.
  */

@@ -244,6 +250,10 @@ syscall_error:
 	
 /* Traced system call support */
 syscall_dotrace:
+#ifdef CONFIG_AUDITSYSCALL
+	andi.	r11,r10,(_TIF_SYSCALL_T_OR_A & ~_TIF_SYSCALL_AUDIT)
+	beq	audit_entry
+#endif
 	bl	.save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.do_syscall_trace_enter

@@ -253,6 +263,7 @@ syscall_dotrace:
 	 * for the call number to look up in the table (r0).
 	 */
 	mr	r0,r3
+.Laudit_entry_return:
 	ld	r3,GPR3(r1)
 	ld	r4,GPR4(r1)
 	ld	r5,GPR5(r1)

@@ -264,6 +275,34 @@ syscall_dotrace:
 	ld	r10,TI_FLAGS(r10)
 	b	.Lsyscall_dotrace_cont
 
+#ifdef CONFIG_AUDITSYSCALL
+audit_entry:
+	ld	r4,GPR0(r1)
+	ld	r5,GPR3(r1)
+	ld	r6,GPR4(r1)
+	ld	r7,GPR5(r1)
+	ld	r8,GPR6(r1)
+
+	andi.	r11,r10,_TIF_32BIT
+	beq	1f
+
+	lis	r3,AUDIT_ARCH_PPC@h
+	ori	r3,r3,AUDIT_ARCH_PPC@l
+	clrldi	r5,r5,32
+	clrldi	r6,r6,32
+	clrldi	r7,r7,32
+	clrldi	r8,r8,32
+	bl	.__audit_syscall_entry
+	ld	r0,GPR0(r1)
+	b	.Laudit_entry_return
+
+1:	lis	r3,AUDIT_ARCH_PPC64@h
+	ori	r3,r3,AUDIT_ARCH_PPC64@l
+	bl	.__audit_syscall_entry
+	ld	r0,GPR0(r1)
+	b	.Laudit_entry_return
+#endif
+
 syscall_enosys:
 	li	r3,-ENOSYS
 	b	syscall_exit

Eric Paris

maintainer

2013-04-10 16:56 UTC #3 lore raw

Re: [PATCH 3/4] powerpc: Optimise 64bit syscall auditing entry path

Patches 1 and 2 I applied for 3.10, but I'd really like to have someone who knows PPC ack 3 and 4. Especially if there is a hope that it goes through my tree...

Link to original messages for your ease of review...

http://marc.info/?l=linux-kernel&m=135768892320439&w=2
http://marc.info/?l=linux-kernel&m=135768895320472&w=2

-Eric

----- Original Message -----

Quoted 96 lines

Add an assembly fast path for the syscall audit entry path on 64bit. Some distros enable auditing by default which forces us through the syscall auditing path even if there are no rules.
I wrote some test cases to validate the patch:
http://ozlabs.org/~anton/junkcode/audit_tests.tar.gz
And to test the performance I ran a simple null syscall microbenchmark on a POWER7 box:
http://ozlabs.org/~anton/junkcode/null_syscall.c
Baseline: 949.2 cycles
Patched:  920.6 cycles
An improvement of 3%. Most of the potential gains are masked by the syscall audit exit path which will be fixed in a subsequent patch.
Signed-off-by Anton Blanchard <[email protected]>
---
Index: b/arch/powerpc/kernel/entry_64.S
===================================================================
arch/powerpc/kernel/entry_64.S +39 -0
@@ -34,6 +34,12 @@
 #include <asm/ftrace.h>
 #include <asm/hw_irq.h>
 
+/* Avoid __ASSEMBLER__'ifying <linux/audit.h> just for this.  */
+#include <linux/elf-em.h>
+#define AUDIT_ARCH_PPC		(EM_PPC)
+#define AUDIT_ARCH_PPC64	(EM_PPC64|__AUDIT_ARCH_64BIT)
+#define __AUDIT_ARCH_64BIT 0x80000000
+
 /*
  * System calls.
  */
@@ -244,6 +250,10 @@ syscall_error:
 
 /* Traced system call support */
 syscall_dotrace:
+#ifdef CONFIG_AUDITSYSCALL
+	andi.	r11,r10,(_TIF_SYSCALL_T_OR_A & ~_TIF_SYSCALL_AUDIT)
+	beq	audit_entry
+#endif
 	bl	.save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.do_syscall_trace_enter
@@ -253,6 +263,7 @@ syscall_dotrace:
 	 * for the call number to look up in the table (r0).
 	 */
 	mr	r0,r3
+.Laudit_entry_return:
 	ld	r3,GPR3(r1)
 	ld	r4,GPR4(r1)
 	ld	r5,GPR5(r1)
@@ -264,6 +275,34 @@ syscall_dotrace:
 	ld	r10,TI_FLAGS(r10)
 	b	.Lsyscall_dotrace_cont
 
+#ifdef CONFIG_AUDITSYSCALL
+audit_entry:
+	ld	r4,GPR0(r1)
+	ld	r5,GPR3(r1)
+	ld	r6,GPR4(r1)
+	ld	r7,GPR5(r1)
+	ld	r8,GPR6(r1)
+
+	andi.	r11,r10,_TIF_32BIT
+	beq	1f
+
+	lis	r3,AUDIT_ARCH_PPC@h
+	ori	r3,r3,AUDIT_ARCH_PPC@l
+	clrldi	r5,r5,32
+	clrldi	r6,r6,32
+	clrldi	r7,r7,32
+	clrldi	r8,r8,32
+	bl	.__audit_syscall_entry
+	ld	r0,GPR0(r1)
+	b	.Laudit_entry_return
+
+1:	lis	r3,AUDIT_ARCH_PPC64@h
+	ori	r3,r3,AUDIT_ARCH_PPC64@l
+	bl	.__audit_syscall_entry
+	ld	r0,GPR0(r1)
+	b	.Laudit_entry_return
+#endif
+
 syscall_enosys:
 	li	r3,-ENOSYS
 	b	syscall_exit

Anton Blanchard

author

2013-01-08 23:48 UTC #4 lore raw

PATCH 4/4 powerpc: Optimise 64bit syscall auditing exit path

In reply to Anton Blanchard

Add an assembly fast path for the syscall audit exit path on 64bit. Some distros enable auditing by default which forces us through the syscall auditing path even if there are no rules.

With syscall auditing enabled we currently disable interrupts, check the threadinfo flags then immediately re-enable interrupts and call audit_syscall_exit. This patch splits the threadinfo flag check into two so we can avoid the disable/reenable of interrupts when handling trace flags. We must do the user work flag check with interrupts off to avoid returning to userspace without handling them.

The other big gain is that we don't have to save and restore the non volatile registers or exit via the slow ret_from_except path.

I wrote some test cases to validate the patch:

http://ozlabs.org/~anton/junkcode/audit_tests.tar.gz

And to test the performance I ran a simple null syscall microbenchmark on a POWER7 box:

http://ozlabs.org/~anton/junkcode/null_syscall.c

Baseline: 920.6 cycles
Patched:  719.6 cycles

An improvement of 22%.

Signed-off-by Anton Blanchard <[email protected]>

---

arch/powerpc/kernel/entry_64.S +64 ~~-32~~

Index: b/arch/powerpc/kernel/entry_64.S
===================================================================
@@ -195,6 +195,19 @@ syscall_exit:
 	andi.	r10,r8,MSR_RI
 	beq-	unrecov_restore
 #endif
+
+	/* We can handle some thread info flags with interrupts on */
+	ld	r9,TI_FLAGS(r12)
+	li	r11,-_LAST_ERRNO
+	andi.   r0,r9,(_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP|_TIF_PERSYSCALL_MASK)
+	bne	syscall_exit_work
+
+	cmpld	r3,r11
+	ld	r5,_CCR(r1)
+	bge-	syscall_error
+
+.Lsyscall_exit_work_cont:
+
 	/*
 	 * Disable interrupts so current_thread_info()->flags can't change,
 	 * and so that we don't get interrupted after loading SRR0/1.

@@ -208,21 +221,19 @@ syscall_exit:
 	 * clear EE. We only need to clear RI just before we restore r13
 	 * below, but batching it with EE saves us one expensive mtmsrd call.
 	 * We have to be careful to restore RI if we branch anywhere from
-	 * here (eg syscall_exit_work).
+	 * here (eg syscall_exit_user_work).
 	 */
 	li	r9,MSR_RI
 	andc	r11,r10,r9
 	mtmsrd	r11,1
 #endif /* CONFIG_PPC_BOOK3E */
 
+	/* Recheck thread info flags with interrupts off */
 	ld	r9,TI_FLAGS(r12)
-	li	r11,-_LAST_ERRNO
-	andi.	r0,r9,(_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
-	bne-	syscall_exit_work
-	cmpld	r3,r11
-	ld	r5,_CCR(r1)
-	bge-	syscall_error
-.Lsyscall_error_cont:
+
+	andi.   r0,r9,_TIF_USER_WORK_MASK
+	bne-	syscall_exit_user_work
+
 	ld	r7,_NIP(r1)
 BEGIN_FTR_SECTION
 	stdcx.	r0,0,r1			/* to clear the reservation */

@@ -246,7 +257,7 @@ syscall_error:
 	oris	r5,r5,0x1000	/* Set SO bit in CR */
 	neg	r3,r3
 	std	r5,_CCR(r1)
-	b	.Lsyscall_error_cont
+	b	.Lsyscall_exit_work_cont
 	
 /* Traced system call support */
 syscall_dotrace:

@@ -306,58 +317,79 @@ audit_entry:
 syscall_enosys:
 	li	r3,-ENOSYS
 	b	syscall_exit
-	
+
 syscall_exit_work:
-#ifdef CONFIG_PPC_BOOK3S
-	mtmsrd	r10,1		/* Restore RI */
-#endif
-	/* If TIF_RESTOREALL is set, don't scribble on either r3 or ccr.
-	 If TIF_NOERROR is set, just save r3 as it is. */
+	li	r6,1		/* r6 contains syscall success */
+	mr	r7,r3
+	ld	r5,_CCR(r1)
 
+	/*
+	 * If TIF_RESTOREALL is set, don't scribble on either r3 or ccr.
+	 * If TIF_NOERROR is set, just save r3 as it is.
+	 */
 	andi.	r0,r9,_TIF_RESTOREALL
 	beq+	0f
 	REST_NVGPRS(r1)
 	b	2f
-0:	cmpld	r3,r11		/* r10 is -LAST_ERRNO */
+0:	cmpld	r3,r11		/* r11 is -LAST_ERRNO */
 	blt+	1f
 	andi.	r0,r9,_TIF_NOERROR
 	bne-	1f
-	ld	r5,_CCR(r1)
+	li	r6,0		/* syscall failed */
 	neg	r3,r3
 	oris	r5,r5,0x1000	/* Set SO bit in CR */
 	std	r5,_CCR(r1)
 1:	std	r3,GPR3(r1)
-2:	andi.	r0,r9,(_TIF_PERSYSCALL_MASK)
+
+2:	andi.	r0,r9,_TIF_SYSCALL_AUDIT
 	beq	4f
 
-	/* Clear per-syscall TIF flags if any are set.  */
+	mr	r3,r6
+	mr	r4,r7
+	bl	.__audit_syscall_exit
+	CURRENT_THREAD_INFO(r12, r1)
+	ld	r9,TI_FLAGS(r12)
+	ld	r3,GPR3(r1)
+	ld	r5,_CCR(r1)
+	ld	r8,_MSR(r1)
+
+4:	andi.	r0,r9,(_TIF_PERSYSCALL_MASK)
+	beq	6f
 
+	/* Clear per-syscall TIF flags if any are set.  */
 	li	r11,_TIF_PERSYSCALL_MASK
 	addi	r12,r12,TI_FLAGS
-3:	ldarx	r10,0,r12
+5:	ldarx	r10,0,r12
 	andc	r10,r10,r11
 	stdcx.	r10,0,r12
-	bne-	3b
+	bne-	5b
 	subi	r12,r12,TI_FLAGS
 
-4:	/* Anything else left to do? */
-	andi.	r0,r9,(_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP)
-	beq	.ret_from_except_lite
+	/*
+	 * We can use the fast path if no other trace flags are on and
+	 * _TIF_RESTOREALL wasn't set.
+	 */
+6:      andi.   r0,r9,((_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP|_TIF_RESTOREALL) & ~_TIF_SYSCALL_AUDIT)
+	mr	r9,r10
+	beq	.Lsyscall_exit_work_cont
 
-	/* Re-enable interrupts */
-#ifdef CONFIG_PPC_BOOK3E
-	wrteei	1
-#else
-	ld	r10,PACAKMSR(r13)
-	ori	r10,r10,MSR_EE
-	mtmsrd	r10,1
-#endif /* CONFIG_PPC_BOOK3E */
+	andi.	r0,r9,((_TIF_SYSCALL_T_OR_A|_TIF_SINGLESTEP) & ~_TIF_SYSCALL_AUDIT)
+	beq	7f
 
 	bl	.save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.do_syscall_trace_leave
 	b	.ret_from_except
 
+7:	b	.ret_from_except_lite
+
+syscall_exit_user_work:
+#ifdef CONFIG_PPC_BOOK3S
+	mtmsrd	r10,1		/* Restore RI */
+#endif
+	std	r3,GPR3(r1)
+	b	.ret_from_except_lite
+
 /* Save non-volatile GPRs, if not already saved. */
 _GLOBAL(save_nvgprs)
 	ld	r11,_TRAP(r1)

arch/powerpc/kernel/ptrace.c +2 -0

Index: b/arch/powerpc/kernel/ptrace.c
===================================================================
@@ -1781,7 +1781,9 @@ void do_syscall_trace_leave(struct pt_re
 {
 	int step;
 
+#ifdef CONFIG_PPC32
 	audit_syscall_exit(regs);
+#endif
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
 		trace_sys_exit(regs, regs->result);

Anton Blanchard

author

2013-02-07 04:13 UTC #5 lore raw

In reply to Anton Blanchard

Hi,

Just following up on this. I've had a few people complaining about audit being broken on ppc64 and it would be nice to fix.

Anton
--

Quoted 38 lines · On Wed, 9 Jan 2013 10:46:17 +1100 Anton Blanchard <[email protected]> wrote:

Commit b05d8447e782 (audit: inline audit_syscall_entry to reduce burden on archs) changed audit_syscall_entry to check for a dummy context before calling __audit_syscall_entry. Unfortunately the dummy context state is maintained in __audit_syscall_entry so once set it never gets cleared, even if the audit rules change.
As a result, if there are no auditing rules when a process starts then it will never be subject to any rules added later. x86 doesn't see this because it has an assembly fast path that calls directly into __audit_syscall_entry.
I noticed this issue when working on audit performance optimisations. I wrote a set of simple test cases available at:
http://ozlabs.org/~anton/junkcode/audit_tests.tar.gz
02_new_rule.py fails without the patch and passes with it. The test case clears all rules, starts a process, adds a rule then verifies the process produces a syscall audit record.
Signed-off-by Anton Blanchard <[email protected]>
Cc <[email protected]> # 3.3+
---
Index: b/include/linux/audit.h
===================================================================
include/linux/audit.h +1 -1
@@ -119,7 +119,7 @@ static inline void audit_syscall_entry(i
 				       unsigned long a1, unsigned
 long a2, unsigned long a3)
 {
-	if (unlikely(!audit_dummy_context()))
+	if (unlikely(current->audit_context))
 		__audit_syscall_entry(arch, major, a0, a1, a2, a3);
 }
 static inline void audit_syscall_exit(void *pt_regs)