[PATCH 3/4] ARCH: AUDIT: implement syscall_get_arch for all arches
For all arches which support audit implement syscall_get_arch()
They are all pretty easy and straight forward, stolen from how the call
to audit_syscall_entry() determines the arch.
Signed-off-by: Eric Paris epa...@redhat.com
Cc: linux-i...@vger.kernel.org
Cc: microblaze-ucli...@itee.uq.edu.au
Cc: linux-m...@linux-mips.org
Cc: li...@lists.openrisc.net
Cc: linux-par...@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparcli...@vger.kernel.org
---
arch/ia64/include/asm/syscall.h | 6 ++
arch/microblaze/include/asm/syscall.h | 5 +
arch/mips/include/asm/syscall.h | 2 +-
arch/openrisc/include/asm/syscall.h | 5 +
arch/parisc/include/asm/syscall.h | 11 +++
arch/powerpc/include/asm/syscall.h| 12
arch/sparc/include/asm/syscall.h | 8
include/uapi/linux/audit.h| 1 +
8 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/arch/ia64/include/asm/syscall.h b/arch/ia64/include/asm/syscall.h
index a7ff1c6..1d0b875 100644
--- a/arch/ia64/include/asm/syscall.h
+++ b/arch/ia64/include/asm/syscall.h
@@ -13,6 +13,7 @@
#ifndef _ASM_SYSCALL_H
#define _ASM_SYSCALL_H 1
+#include uapi/linux/audit.h
#include linux/sched.h
#include linux/err.h
@@ -79,4 +80,9 @@ static inline void syscall_set_arguments(struct task_struct
*task,
ia64_syscall_get_set_arguments(task, regs, i, n, args, 1);
}
+
+static inline int syscall_get_arch(void)
+{
+ return AUDIT_ARCH_IA64;
+}
#endif /* _ASM_SYSCALL_H */
diff --git a/arch/microblaze/include/asm/syscall.h
b/arch/microblaze/include/asm/syscall.h
index 9bc4317..53cfaf3 100644
--- a/arch/microblaze/include/asm/syscall.h
+++ b/arch/microblaze/include/asm/syscall.h
@@ -1,6 +1,7 @@
#ifndef __ASM_MICROBLAZE_SYSCALL_H
#define __ASM_MICROBLAZE_SYSCALL_H
+#include uapi/linux/audit.h
#include linux/kernel.h
#include linux/sched.h
#include asm/ptrace.h
@@ -99,4 +100,8 @@ static inline void syscall_set_arguments(struct task_struct
*task,
asmlinkage long do_syscall_trace_enter(struct pt_regs *regs);
asmlinkage void do_syscall_trace_leave(struct pt_regs *regs);
+static inline int syscall_get_arch(void)
+{
+ return AUDIT_ARCH_MICROBLAZE;
+}
#endif /* __ASM_MICROBLAZE_SYSCALL_H */
diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
index fc556d8..992b6ab 100644
--- a/arch/mips/include/asm/syscall.h
+++ b/arch/mips/include/asm/syscall.h
@@ -103,7 +103,7 @@ extern const unsigned long sysn32_call_table[];
static inline int syscall_get_arch(void)
{
- int arch = EM_MIPS;
+ int arch = AUDIT_ARCH_MIPS;
#ifdef CONFIG_64BIT
arch |= __AUDIT_ARCH_64BIT;
#endif
diff --git a/arch/openrisc/include/asm/syscall.h
b/arch/openrisc/include/asm/syscall.h
index b752bb6..2db9f1c 100644
--- a/arch/openrisc/include/asm/syscall.h
+++ b/arch/openrisc/include/asm/syscall.h
@@ -19,6 +19,7 @@
#ifndef __ASM_OPENRISC_SYSCALL_H__
#define __ASM_OPENRISC_SYSCALL_H__
+#include uapi/linux/audit.h
#include linux/err.h
#include linux/sched.h
@@ -71,4 +72,8 @@ syscall_set_arguments(struct task_struct *task, struct
pt_regs *regs,
memcpy(regs-gpr[3 + i], args, n * sizeof(args[0]));
}
+static inline int syscall_get_arch(void)
+{
+ return AUDIT_ARCH_OPENRISC;
+}
#endif
diff --git a/arch/parisc/include/asm/syscall.h
b/arch/parisc/include/asm/syscall.h
index 8bdfd2c..a5eba95 100644
--- a/arch/parisc/include/asm/syscall.h
+++ b/arch/parisc/include/asm/syscall.h
@@ -3,6 +3,8 @@
#ifndef _ASM_PARISC_SYSCALL_H_
#define _ASM_PARISC_SYSCALL_H_
+#include uapi/linux/audit.h
+#include linux/compat.h
#include linux/err.h
#include asm/ptrace.h
@@ -37,4 +39,13 @@ static inline void syscall_get_arguments(struct task_struct
*tsk,
}
}
+static inline int syscall_get_arch(void)
+{
+ int arch = AUDIT_ARCH_PARISC;
+#ifdef CONFIG_64BIT
+ if (!is_compat_task())
+ arch = AUDIT_ARCH_PARISC64;
+#endif
+ return arch;
+}
#endif /*_ASM_PARISC_SYSCALL_H_*/
diff --git a/arch/powerpc/include/asm/syscall.h
b/arch/powerpc/include/asm/syscall.h
index b54b2ad..4271544 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -13,6 +13,8 @@
#ifndef _ASM_SYSCALL_H
#define _ASM_SYSCALL_H 1
+#include uapi/linux/audit.h
+#include linux/compat.h
#include linux/sched.h
/* ftrace syscalls requires exporting the sys_call_table */
@@ -86,4 +88,14 @@ static inline void syscall_set_arguments(struct task_struct
*task,
memcpy(regs-gpr[3 + i], args, n * sizeof(args[0]));
}
+static inline int syscall_get_arch(void)
+{
+ int arch = AUDIT_ARCH_PPC;
+
+#ifdef CONFIG_PPC64
+ if (!is_32bit_task())
+ arch = AUDIT_ARCH_PPC64;
+#endif
+ return arch;
+}
#endif /* _ASM_SYSCALL_H */
diff --git a/arch/sparc/include/asm/syscall.h b/arch/sparc/include/asm/syscall.h
index 025a02a..fed3d51 100644
--- a/arch/sparc/include/asm/syscall.h
+++ b/arch/sparc
[PATCH 4/4] ARCH: AUDIT: audit_syscall_entry() should not require the arch
We have a function where the arch can be queried, syscall_get_arch(). So rather than have every single piece of arch specific code use and/or duplicate syscall_get_arch(), just have the audit code use the syscall_get_arch() code. Signed-off-by: Eric Paris epa...@redhat.com Cc: linux-al...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linux-i...@vger.kernel.org Cc: microblaze-ucli...@itee.uq.edu.au Cc: linux-m...@linux-mips.org Cc: li...@lists.openrisc.net Cc: linux-par...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-s...@vger.kernel.org Cc: linux...@vger.kernel.org Cc: sparcli...@vger.kernel.org Cc: user-mode-linux-de...@lists.sourceforge.net Cc: linux-xte...@linux-xtensa.org Cc: x...@kernel.org --- arch/alpha/kernel/ptrace.c | 2 +- arch/arm/kernel/ptrace.c| 4 ++-- arch/ia64/kernel/ptrace.c | 2 +- arch/microblaze/kernel/ptrace.c | 3 +-- arch/mips/kernel/ptrace.c | 4 +--- arch/openrisc/kernel/ptrace.c | 3 +-- arch/parisc/kernel/ptrace.c | 9 +++-- arch/powerpc/kernel/ptrace.c| 7 ++- arch/s390/kernel/ptrace.c | 4 +--- arch/sh/kernel/ptrace_32.c | 14 +- arch/sh/kernel/ptrace_64.c | 17 + arch/sparc/kernel/ptrace_64.c | 9 ++--- arch/um/kernel/ptrace.c | 3 +-- arch/x86/kernel/ptrace.c| 8 ++-- arch/x86/um/asm/ptrace.h| 4 arch/xtensa/kernel/ptrace.c | 2 +- include/linux/audit.h | 7 --- 17 files changed, 25 insertions(+), 77 deletions(-) diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c index 86d8351..d9ee817 100644 --- a/arch/alpha/kernel/ptrace.c +++ b/arch/alpha/kernel/ptrace.c @@ -321,7 +321,7 @@ asmlinkage unsigned long syscall_trace_enter(void) if (test_thread_flag(TIF_SYSCALL_TRACE) tracehook_report_syscall_entry(current_pt_regs())) ret = -1UL; - audit_syscall_entry(AUDIT_ARCH_ALPHA, regs-r0, regs-r16, regs-r17, regs-r18, regs-r19); + audit_syscall_entry(regs-r0, regs-r16, regs-r17, regs-r18, regs-r19); return ret ?: current_pt_regs()-r0; } diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c index 0dd3b79..c9d2b34 100644 --- a/arch/arm/kernel/ptrace.c +++ b/arch/arm/kernel/ptrace.c @@ -943,8 +943,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno) if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, scno); - audit_syscall_entry(AUDIT_ARCH_ARM, scno, regs-ARM_r0, regs-ARM_r1, - regs-ARM_r2, regs-ARM_r3); + audit_syscall_entry(scno, regs-ARM_r0, regs-ARM_r1, regs-ARM_r2, + regs-ARM_r3); return scno; } diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c index b7a5fff..6f54d51 100644 --- a/arch/ia64/kernel/ptrace.c +++ b/arch/ia64/kernel/ptrace.c @@ -1219,7 +1219,7 @@ syscall_trace_enter (long arg0, long arg1, long arg2, long arg3, ia64_sync_krbs(); - audit_syscall_entry(AUDIT_ARCH_IA64, regs.r15, arg0, arg1, arg2, arg3); + audit_syscall_entry(regs.r15, arg0, arg1, arg2, arg3); return 0; } diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c index 39cf508..bb10637 100644 --- a/arch/microblaze/kernel/ptrace.c +++ b/arch/microblaze/kernel/ptrace.c @@ -147,8 +147,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) */ ret = -1L; - audit_syscall_entry(EM_MICROBLAZE, regs-r12, regs-r5, regs-r6, - regs-r7, regs-r8); + audit_syscall_entry(regs-r12, regs-r5, regs-r6, regs-r7, regs-r8); return ret ?: regs-r12; } diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c index 65ba622..c06bb82 100644 --- a/arch/mips/kernel/ptrace.c +++ b/arch/mips/kernel/ptrace.c @@ -671,9 +671,7 @@ asmlinkage void syscall_trace_enter(struct pt_regs *regs) if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_enter(regs, regs-regs[2]); - audit_syscall_entry(syscall_get_arch(), - regs-regs[2], - regs-regs[4], regs-regs[5], + audit_syscall_entry(regs-regs[2], regs-regs[4], regs-regs[5], regs-regs[6], regs-regs[7]); } diff --git a/arch/openrisc/kernel/ptrace.c b/arch/openrisc/kernel/ptrace.c index 71a2a0c..4f59fa4 100644 --- a/arch/openrisc/kernel/ptrace.c +++ b/arch/openrisc/kernel/ptrace.c @@ -187,8 +187,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) */ ret = -1L; - audit_syscall_entry(AUDIT_ARCH_OPENRISC, regs-gpr[11], - regs-gpr[3], regs-gpr[4], + audit_syscall_entry(regs-gpr[11], regs-gpr[3], regs-gpr[4], regs-gpr[5], regs-gpr[6]); return ret ? : regs-gpr[11
Re: [PATCH 3/4] ARCH: AUDIT: implement syscall_get_arch for all arches
On Wed, 2014-03-19 at 15:19 -0700, Matt Turner wrote: On Wed, Mar 19, 2014 at 3:04 PM, Eric Paris epa...@redhat.com wrote: For all arches which support audit implement syscall_get_arch() support audit -- is that AUDIT_ARCH? If so, alpha gained support recently, so I think this patch needs to handle alpha too? Absolutely right. I broke Alpha (in the next patch). Will fix. -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
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-kernelm=135768892320439w=2 http://marc.info/?l=linux-kernelm=135768895320472w=2 -Eric - Original Message - 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 an...@samba.org --- Index: b/arch/powerpc/kernel/entry_64.S === --- a/arch/powerpc/kernel/entry_64.S +++ 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 0x8000 + /* * 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 addir3,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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH -v3] Audit: push audit success and retcode into arch ptrace.h
The audit system previously expected arches calling to audit_syscall_exit to
supply as arguments if the syscall was a success and what the return code was.
Audit also provides a helper AUDITSC_RESULT which was supposed to simplify
things
by converting from negative retcodes to an audit internal magic value stating
success or failure. This helper was wrong and could indicate that a valid
pointer returned to userspace was a failed syscall. The fix is to fix the
layering foolishness. We now pass audit_syscall_exit a struct pt_reg and it
in turns calls back into arch code to collect the return value and to
determine if the syscall was a success or failure. We also define a generic
is_syscall_success() macro which determines success/failure based on if the
value is -MAX_ERRNO. This works for arches like x86 which do not use a
separate mechanism to indicate syscall failure.
We make both the is_syscall_success() and regs_return_value() static inlines
instead of macros. The reason is because the audit function must take a void*
for the regs. (uml calls theirs struct uml_pt_regs instead of just struct
pt_regs so audit_syscall_exit can't take a struct pt_regs). Since the audit
function takes a void* we need to use static inlines to cast it back to the
arch correct structure to dereference it.
The other major change is that on some arches, like ia64, MIPS and ppc, we
change regs_return_value() to give us the negative value on syscall failure.
THE only other user of this macro, kretprobe_example.c, won't notice and it
makes the value signed consistently for the audit functions across all archs.
In arch/sh/kernel/ptrace_64.c I see that we were using regs[9] in the old
audit code as the return value. But the ptrace_64.h code defined the macro
regs_return_value() as regs[3]. I have no idea which one is correct, but this
patch now uses the regs_return_value() function, so it now uses regs[3].
For powerpc we previously used regs-result but now use the
regs_return_value() function which uses regs-gprs[3]. regs-gprs[3] is
always positive so the regs_return_value(), much like ia64 makes it negative
before calling the audit code when appropriate.
Signed-off-by: Eric Paris epa...@redhat.com
Acked-by: H. Peter Anvin h...@zytor.com [for x86 portion]
Acked-by: Tony Luck tony.l...@intel.com [for ia64]
Acked-by: Richard Weinberger rich...@nod.at [for uml]
Acked-by: David S. Miller da...@davemloft.net [for sparc]
Acked-by: Ralf Baechle r...@linux-mips.org [for mips]
Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org [for ppc]
---
arch/ia64/include/asm/ptrace.h| 13 -
arch/ia64/kernel/ptrace.c |9 +
arch/microblaze/include/asm/ptrace.h |5 +
arch/microblaze/kernel/ptrace.c |3 +--
arch/mips/include/asm/ptrace.h| 14 +-
arch/mips/kernel/ptrace.c |4 +---
arch/powerpc/include/asm/ptrace.h | 13 -
arch/powerpc/kernel/ptrace.c |4 +---
arch/s390/include/asm/ptrace.h|6 +-
arch/s390/kernel/ptrace.c |4 +---
arch/sh/include/asm/ptrace_32.h |5 -
arch/sh/include/asm/ptrace_64.h |5 -
arch/sh/kernel/ptrace_32.c|4 +---
arch/sh/kernel/ptrace_64.c|4 +---
arch/sparc/include/asm/ptrace.h | 10 +-
arch/sparc/kernel/ptrace_64.c | 11 +--
arch/um/kernel/ptrace.c |4 ++--
arch/um/sys-i386/shared/sysdep/ptrace.h |4
arch/um/sys-x86_64/shared/sysdep/ptrace.h |4
arch/x86/ia32/ia32entry.S | 10 +-
arch/x86/kernel/entry_32.S|8
arch/x86/kernel/entry_64.S| 10 +-
arch/x86/kernel/ptrace.c |3 +--
arch/x86/kernel/vm86_32.c |4 ++--
include/linux/audit.h | 22 ++
include/linux/ptrace.h| 10 ++
kernel/auditsc.c | 16
27 files changed, 135 insertions(+), 74 deletions(-)
diff --git a/arch/ia64/include/asm/ptrace.h b/arch/ia64/include/asm/ptrace.h
index 7ae9c3f..801ee1a 100644
--- a/arch/ia64/include/asm/ptrace.h
+++ b/arch/ia64/include/asm/ptrace.h
@@ -246,7 +246,18 @@ static inline unsigned long user_stack_pointer(struct
pt_regs *regs)
return regs-ar_bspstore;
}
-#define regs_return_value(regs) ((regs)-r8)
+static inline int is_syscall_success(struct pt_regs *regs)
+{
+ return regs-r10 != -1;
+}
+
+static inline long regs_return_value(struct pt_regs *regs)
+{
+ if (is_syscall_success(regs))
+ return regs-r8;
+ else
+ return -regs-r8;
+}
/* Conserve space in histogram by encoding slot bits in address
* bits 2 and 3 rather than bits 0 and 1.
diff --git a/arch/ia64/kernel
Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h
On Wed, 2011-06-08 at 18:36 +0200, Oleg Nesterov wrote: On 06/07, Eric Paris wrote: On Tue, 2011-06-07 at 19:19 +0200, Oleg Nesterov wrote: With or without this patch, can't we call audit_syscall_exit() twice if there is something else in _TIF_WORK_SYSCALL_EXIT mask apart from SYSCALL_AUDIT ? First time it is called from asm, then from syscall_trace_leave(), no? For example. The task has TIF_SYSCALL_AUDIT and nothing else, it does system_call-auditsys-system_call_fastpath. What if it gets, say, TIF_SYSCALL_TRACE before ret_from_sys_call? No harm is done calling twice. The first call will do the real work and cleanup. It will set a flag in the audit data that the work has been done (in_syscall == 0) thus the second call will then not do any real work and won't have anything to clean up. Hmm... and I assume context-previous != NULL is not possible on x86_64. OK, thanks. And I guess, all CONFIG_AUDITSYSCALL code in entry.S is only needed to microoptimize the case when TIF_SYSCALL_AUDIT is the only reason for the slow path. I wonder if it really makes the measureble difference... All I know is what Roland put in the changelog: Avoiding the iret return path when syscall audit is enabled helps performance a lot. I believe this was a result of Fedora starting auditd by default and then Linus bitching about how slow a null syscall in a tight loop was. It was an optimization for a microbenchmark. How much it affects things on a real syscall that does real work is probably going to be determined by how much work is done in the syscall. (or just disable auditd in userspace) -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h
On Tue, 2011-06-07 at 19:19 +0200, Oleg Nesterov wrote: On 06/03, Eric Paris wrote: The audit system previously expected arches calling to audit_syscall_exit to supply as arguments if the syscall was a success and what the return code was. Audit also provides a helper AUDITSC_RESULT which was supposed to simplify things by converting from negative retcodes to an audit internal magic value stating success or failure. This helper was wrong and could indicate that a valid pointer returned to userspace was a failed syscall. The fix is to fix the layering foolishness. We now pass audit_syscall_exit a struct pt_reg and it in turns calls back into arch code to collect the return value and to determine if the syscall was a success or failure. We also define a generic is_syscall_success() macro which determines success/failure based on if the value is -MAX_ERRNO. This works for arches like x86 which do not use a separate mechanism to indicate syscall failure. I know nothing about audit, but the patch looks fine to me. But I have a bit off-topic question, diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S index 8a445a0..b7b1f88 100644 --- a/arch/x86/kernel/entry_64.S +++ b/arch/x86/kernel/entry_64.S @@ -53,6 +53,7 @@ #include asm/paravirt.h #include asm/ftrace.h #include asm/percpu.h +#include linux/err.h /* Avoid __ASSEMBLER__'ifying linux/audit.h just for this. */ #include linux/elf-em.h @@ -564,17 +565,16 @@ auditsys: jmp system_call_fastpath /* -* Return fast path for syscall audit. Call audit_syscall_exit() +* Return fast path for syscall audit. Call __audit_syscall_exit() * directly and then jump back to the fast path with TIF_SYSCALL_AUDIT * masked off. */ sysret_audit: movq RAX-ARGOFFSET(%rsp),%rsi /* second arg, syscall return value */ - cmpq $0,%rsi/* is it 0? */ - setl %al/* 1 if so, 0 if not */ + cmpq $-MAX_ERRNO,%rsi /* is it -MAX_ERRNO? */ + setbe %al /* 1 if so, 0 if not */ movzbl %al,%edi /* zero-extend that into %edi */ - inc %edi /* first arg, 0-1(AUDITSC_SUCCESS), 1-2(AUDITSC_FAILURE) */ - call audit_syscall_exit + call __audit_syscall_exit With or without this patch, can't we call audit_syscall_exit() twice if there is something else in _TIF_WORK_SYSCALL_EXIT mask apart from SYSCALL_AUDIT ? First time it is called from asm, then from syscall_trace_leave(), no? For example. The task has TIF_SYSCALL_AUDIT and nothing else, it does system_call-auditsys-system_call_fastpath. What if it gets, say, TIF_SYSCALL_TRACE before ret_from_sys_call? No harm is done calling twice. The first call will do the real work and cleanup. It will set a flag in the audit data that the work has been done (in_syscall == 0) thus the second call will then not do any real work and won't have anything to clean up. -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH -v2] Audit: push audit success and retcode into arch ptrace.h
The audit system previously expected arches calling to audit_syscall_exit to
supply as arguments if the syscall was a success and what the return code was.
Audit also provides a helper AUDITSC_RESULT which was supposed to simplify
things
by converting from negative retcodes to an audit internal magic value stating
success or failure. This helper was wrong and could indicate that a valid
pointer returned to userspace was a failed syscall. The fix is to fix the
layering foolishness. We now pass audit_syscall_exit a struct pt_reg and it
in turns calls back into arch code to collect the return value and to
determine if the syscall was a success or failure. We also define a generic
is_syscall_success() macro which determines success/failure based on if the
value is -MAX_ERRNO. This works for arches like x86 which do not use a
separate mechanism to indicate syscall failure.
In arch/sh/kernel/ptrace_64.c I see that we were using regs[9] in the old
audit code as the return value. But the ptrace_64.h code defined the macro
regs_return_value() as regs[3]. I have no idea which one is correct, but this
patch now uses the regs_return_value() function, so it now uses regs[3].
We make both the is_syscall_success() and regs_return_value() static inlines
instead of macros. The reason is because the audit function must take a void*
for the regs. (uml calls theirs struct uml_pt_regs instead of just struct
pt_regs so audit_syscall_exit can't take a struct pt_regs). Since the audit
function takes a void* we need to use static inlines to cast it back to the
arch correct structure to dereference it.
The other major change is that on some arches, like ia64, we change
regs_return_value() to give us the negative value on syscall failure. The
only other user of this macro, kretprobe_example.c, won't notice and it makes
the value signed consistently for the audit functions across all archs.
Signed-off-by: Eric Paris epa...@redhat.com
Acked-by: Acked-by: H. Peter Anvin h...@zytor.com [for x86 portion]
---
arch/ia64/include/asm/ptrace.h| 13 -
arch/ia64/kernel/ptrace.c |9 +
arch/microblaze/include/asm/ptrace.h |5 +
arch/microblaze/kernel/ptrace.c |3 +--
arch/mips/include/asm/ptrace.h| 14 +-
arch/mips/kernel/ptrace.c |4 +---
arch/powerpc/include/asm/ptrace.h | 10 +-
arch/powerpc/kernel/ptrace.c |4 +---
arch/s390/include/asm/ptrace.h|6 +-
arch/s390/kernel/ptrace.c |4 +---
arch/sh/include/asm/ptrace_32.h |5 -
arch/sh/include/asm/ptrace_64.h |5 -
arch/sh/kernel/ptrace_32.c|4 +---
arch/sh/kernel/ptrace_64.c|4 +---
arch/sparc/include/asm/ptrace.h | 10 +-
arch/sparc/kernel/ptrace_64.c | 11 +--
arch/um/kernel/ptrace.c |4 ++--
arch/um/sys-i386/shared/sysdep/ptrace.h |4
arch/um/sys-x86_64/shared/sysdep/ptrace.h |4
arch/x86/ia32/ia32entry.S | 10 +-
arch/x86/kernel/entry_32.S|8
arch/x86/kernel/entry_64.S| 10 +-
arch/x86/kernel/ptrace.c |3 +--
arch/x86/kernel/vm86_32.c |4 ++--
include/linux/audit.h | 22 ++
include/linux/ptrace.h| 10 ++
kernel/auditsc.c | 16
27 files changed, 132 insertions(+), 74 deletions(-)
diff --git a/arch/ia64/include/asm/ptrace.h b/arch/ia64/include/asm/ptrace.h
index 7ae9c3f..801ee1a 100644
--- a/arch/ia64/include/asm/ptrace.h
+++ b/arch/ia64/include/asm/ptrace.h
@@ -246,7 +246,18 @@ static inline unsigned long user_stack_pointer(struct
pt_regs *regs)
return regs-ar_bspstore;
}
-#define regs_return_value(regs) ((regs)-r8)
+static inline int is_syscall_success(struct pt_regs *regs)
+{
+ return regs-r10 != -1;
+}
+
+static inline long regs_return_value(struct pt_regs *regs)
+{
+ if (is_syscall_success(regs))
+ return regs-r8;
+ else
+ return -regs-r8;
+}
/* Conserve space in histogram by encoding slot bits in address
* bits 2 and 3 rather than bits 0 and 1.
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 8848f43..2c15408 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1268,14 +1268,7 @@ syscall_trace_leave (long arg0, long arg1, long arg2,
long arg3,
{
int step;
- if (unlikely(current-audit_context)) {
- int success = AUDITSC_RESULT(regs.r10);
- long result = regs.r8;
-
- if (success != AUDITSC_SUCCESS)
- result = -result;
- audit_syscall_exit(success, result
[PATCH] Audit: push audit success and retcode into arch ptrace.h
The audit system previously expected arches calling to audit_syscall_exit to
supply as arguments if the syscall was a success and what the return code was.
Audit also provides a helper AUDITSC_RESULT which was supposed to simplify by
converting from negative retcodes to an audit internal magic value stating
success or failure. This helper was wrong and could indicate that a valid
pointer returned to userspace was a failed syscall. The fix is to fix the
layering foolishness. We now pass audit_syscall_exit a struct pt_reg and it
in turns calls back into arch code to collect the return value and to
determine if the syscall was a success or failure.
In arch/sh/kernel/ptrace_64.c I see that we were using regs[9] in the old
audit code. But the ptrace_64.h code was previously using regs[3] for the
regs_return_value(). I have no idea which one is correct, but this patch now
uses regs[3].
Signed-off-by: Eric Paris epa...@redhat.com
---
arch/ia64/include/asm/ptrace.h| 10 +-
arch/ia64/kernel/ptrace.c |9 +
arch/microblaze/include/asm/ptrace.h |2 ++
arch/microblaze/kernel/ptrace.c |3 +--
arch/mips/include/asm/ptrace.h| 11 ++-
arch/mips/kernel/ptrace.c |4 +---
arch/powerpc/include/asm/ptrace.h |1 +
arch/powerpc/kernel/ptrace.c |4 +---
arch/s390/kernel/ptrace.c |4 +---
arch/sh/kernel/ptrace_32.c|4 +---
arch/sh/kernel/ptrace_64.c|4 +---
arch/sparc/include/asm/ptrace.h |1 +
arch/sparc/kernel/ptrace_64.c | 11 +--
arch/um/kernel/ptrace.c |4 ++--
arch/um/sys-i386/shared/sysdep/ptrace.h |1 +
arch/um/sys-x86_64/shared/sysdep/ptrace.h |1 +
arch/x86/ia32/ia32entry.S | 10 +-
arch/x86/kernel/entry_32.S|8
arch/x86/kernel/entry_64.S| 10 +-
arch/x86/kernel/ptrace.c |3 +--
arch/x86/kernel/vm86_32.c |3 +--
include/linux/audit.h | 22 ++
include/linux/ptrace.h| 10 ++
kernel/auditsc.c | 16
24 files changed, 87 insertions(+), 69 deletions(-)
diff --git a/arch/ia64/include/asm/ptrace.h b/arch/ia64/include/asm/ptrace.h
index 7ae9c3f..9e422e3 100644
--- a/arch/ia64/include/asm/ptrace.h
+++ b/arch/ia64/include/asm/ptrace.h
@@ -246,7 +246,15 @@ static inline unsigned long user_stack_pointer(struct
pt_regs *regs)
return regs-ar_bspstore;
}
-#define regs_return_value(regs) ((regs)-r8)
+#define is_syscall_success(regs) ((regs)-r10 != -1)
+#define regs_return_value(regs)\
+({ \
+ const typeof(regs) *__regs = (regs);\
+ if (is_syscall_success(__regs)) \
+ __regs-r8; \
+ else\
+ -(__regs-r8) \
+})
/* Conserve space in histogram by encoding slot bits in address
* bits 2 and 3 rather than bits 0 and 1.
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 8848f43..2c15408 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1268,14 +1268,7 @@ syscall_trace_leave (long arg0, long arg1, long arg2,
long arg3,
{
int step;
- if (unlikely(current-audit_context)) {
- int success = AUDITSC_RESULT(regs.r10);
- long result = regs.r8;
-
- if (success != AUDITSC_SUCCESS)
- result = -result;
- audit_syscall_exit(success, result);
- }
+ audit_syscall_exit(regs);
step = test_thread_flag(TIF_SINGLESTEP);
if (step || test_thread_flag(TIF_SYSCALL_TRACE))
diff --git a/arch/microblaze/include/asm/ptrace.h
b/arch/microblaze/include/asm/ptrace.h
index d9b6630..c59c209 100644
--- a/arch/microblaze/include/asm/ptrace.h
+++ b/arch/microblaze/include/asm/ptrace.h
@@ -61,6 +61,8 @@ struct pt_regs {
#define instruction_pointer(regs) ((regs)-pc)
#define profile_pc(regs) instruction_pointer(regs)
+#define regs_return_value(regs)((regs)-r3)
+
void show_regs(struct pt_regs *);
#else /* __KERNEL__ */
diff --git a/arch/microblaze/kernel/ptrace.c b/arch/microblaze/kernel/ptrace.c
index 6a8e0cc..9747fa5 100644
--- a/arch/microblaze/kernel/ptrace.c
+++ b/arch/microblaze/kernel/ptrace.c
@@ -159,8 +159,7 @@ asmlinkage void do_syscall_trace_leave(struct pt_regs *regs)
{
int step;
- if (unlikely(current-audit_context))
- audit_syscall_exit(AUDITSC_RESULT(regs-r3), regs-r3
Re: [PATCH] Audit: push audit success and retcode into arch ptrace.h
On 06/02/2011 06:32 PM, Richard Weinberger wrote:
Am Donnerstag 02 Juni 2011, 23:04:58 schrieb Eric Paris:
b/arch/um/sys-i386/shared/sysdep/ptrace.h index d50e62e..ef5c310 100644
--- a/arch/um/sys-i386/shared/sysdep/ptrace.h
+++ b/arch/um/sys-i386/shared/sysdep/ptrace.h
@@ -162,6 +162,7 @@ struct syscall_args {
#define UPT_ORIG_SYSCALL(r) UPT_EAX(r)
#define UPT_SYSCALL_NR(r) UPT_ORIG_EAX(r)
#define UPT_SYSCALL_RET(r) UPT_EAX(r)
+#define regs_return_value UPT_SYSCALL_RET
This does not work at all.
UPT_SYSCALL_RET expects something of type struct uml_pt_regs.
#define regs_return_value REGS_EAX
Would be correct. (For x86_64 it needs to be REGS_RAX)
But there seems to be another problem.
Why is pt_regs of type void *?
I was stupid and used #define's instead of static inlines. Sorry. I
wonder how many other arches I got that wrong, i'm sure others
The code in arch/um/kernel/ptrace.c::syscall_trace() appeared to have a
uml_pt_regs instead of just pt_regs. Which was why audit_syscall_exit()
takes a void * instead of a pt_regs. We pass that right back to
regs_return_value(). I believe the correct code should be:
static inline long regs_return_value(struct uml_pt_regs *r)
{
return UPT_SYSCALL_RET(r);
}
gcc complains:
In file included from include/linux/fsnotify.h:15:0,
from include/linux/security.h:26,
from init/main.c:32:
include/linux/audit.h: In function ‘audit_syscall_exit’:
include/linux/audit.h:440:17: warning: dereferencing ‘void *’ pointer
include/linux/audit.h:440:3: error: invalid use of void expression
include/linux/audit.h:441:21: warning: dereferencing ‘void *’ pointer
include/linux/audit.h:441:21: error: void value not ignored as it ought to be
Thanks,
//richard
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Audit: push audit success and retcode into arch ptrace.h
On 06/02/2011 07:00 PM, Tony Luck wrote: But there seems to be another problem. Why is pt_regs of type void *? gcc complains: In file included from include/linux/fsnotify.h:15:0, from include/linux/security.h:26, from init/main.c:32: include/linux/audit.h: In function ‘audit_syscall_exit’: include/linux/audit.h:440:17: warning: dereferencing ‘void *’ pointer include/linux/audit.h:440:3: error: invalid use of void expression include/linux/audit.h:441:21: warning: dereferencing ‘void *’ pointer include/linux/audit.h:441:21: error: void value not ignored as it ought to be Perhaps same issue on ia64 - but symptoms are different: CC crypto/cipher.o In file included from include/linux/fsnotify.h:15, from include/linux/security.h:26, from init/do_mounts.c:8: include/linux/audit.h: In function ‘audit_syscall_exit’: include/linux/audit.h:440: warning: dereferencing ‘void *’ pointer include/linux/audit.h:440: error: request for member ‘r10’ in something not a structure or union include/linux/audit.h:441: error: request for member ‘r10’ in something not a structure or union include/linux/audit.h:441: error: request for member ‘r8’ in something not a structure or union include/linux/audit.h:441: error: request for member ‘r8’ in something not a structure or union include/linux/audit.h:441: error: expected ‘;’ before ‘}’ token include/linux/audit.h:441: error: void value not ignored as it ought to be I think it is the same problem. I'll redo the patch with typed static inlines instead of #defines and all of this should hopefully work out. -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/5] v2 seccomp_filters: Enable ftrace-based system call filtering
[dropping microblaze and roland] lOn Fri, 2011-05-13 at 14:10 +0200, Ingo Molnar wrote: * James Morris jmor...@namei.org wrote: It is a simple and sensible security feature, agreed? It allows most code to run well and link to countless libraries - but no access to other files is allowed. It's simple enough and sounds reasonable, but you can read all the discussion about AppArmour why many people don't really think it's the best. Still, I'll agree it's a lot better than nothing. But if i had a VFS event at the fs/namei.c::getname() level, i would have access to a central point where the VFS string becomes stable to the kernel and can be checked (and denied if necessary). A sidenote, and not surprisingly, the audit subsystem already has an event callback there: audit_getname(result); Unfortunately this audit callback cannot be used for my purposes, because the event is single-purpose for auditd and because it allows no feedback (no deny/accept discretion for the security policy). But if had this simple event there: err = event_vfs_getname(result); Wow it sounds so easy. Now lets keep extending your train of thought until we can actually provide the security provided by SELinux. What do we end up with? We end up with an event hook right next to every LSM hook. You know, the LSM hooks were placed where they are for a reason. Because those were the locations inside the kernel where you actually have information about the task doing an operation and the objects (files, sockets, directories, other tasks, etc) they are doing an operation on. Honestly all you are talking about it remaking the LSM with 2 sets of hooks instead if 1. Why? It seems much easier that if you want the language of the filter engine you would just make a new LSM that uses the filter engine for it's policy language rather than the language created by SELinux or SMACK or name your LSM implementation. - unprivileged: application-definable, allowing the embedding of security policy in *apps* as well, not just the system - flexible: can be added/removed runtime unprivileged, and cheaply so - transparent: does not impact executing code that meets the policy - nestable: it is inherited by child tasks and is fundamentally stackable, multiple policies will have the combined effect and they are transparent to each other. So if a child task within a sandbox adds *more* checks then those add to the already existing set of checks. We only narrow permissions, never extend them. - generic: allowing observation and (safe) control of security relevant parameters not just at the system call boundary but at other relevant places of kernel execution as well: which points/callbacks could also be used for other types of event extraction such as perf. It could even be shared with audit ... I'm not arguing that any of these things are bad things. What you describe is a new LSM that uses a discretionary access control model but with the granularity and flexibility that has traditionally only existed in the mandatory access control security modules previously implemented in the kernel. I won't argue that's a bad idea, there's no reason in my mind that a process shouldn't be allowed to control it's own access decisions in a more flexible way than rwx bits. Then again, I certainly don't see a reason that this syscall hardening patch should be held up while a whole new concept in computer security is contemplated... -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/5] v2 seccomp_filters: Enable ftrace-based system call filtering
[dropping microblaze and roland] On Fri, 2011-05-13 at 15:18 +0200, Ingo Molnar wrote: * Peter Zijlstra pet...@infradead.org wrote: On Fri, 2011-05-13 at 14:54 +0200, Ingo Molnar wrote: I think the sanest semantics is to run all active callbacks as well. For example if this is used for three stacked security policies - as if 3 LSM modules were stacked at once. We'd call all three, and we'd determine that at least one failed - and we'd return a failure. But that only works for boolean functions where you can return the multi-bit-or of the result. What if you need to return the specific error code. Do you mean that one filter returns -EINVAL while the other -EACCES? Seems like a non-problem to me, we'd return the first nonzero value. Sounds so easy! Why haven't LSMs stacked already? Because what happens if one of these hooks did something stateful? Lets say on open, hook #1 returns EPERM. hook #2 allocates memory. The open is going to fail and hooks #2 is never going to get the close() which should have freed the allocation. If you can be completely stateless its easier, but there's a reason that stacking security modules is hard. Serge has tried in the past and both dhowells and casey schaufler are working on it right now. Stacking is never as easy as it sounds :) -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/5] v2 seccomp_filters: Enable ftrace-based system call filtering
On Fri, 2011-05-13 at 17:23 +0200, Peter Zijlstra wrote: On Fri, 2011-05-13 at 11:10 -0400, Eric Paris wrote: Then again, I certainly don't see a reason that this syscall hardening patch should be held up while a whole new concept in computer security is contemplated... Which makes me wonder why this syscall hardening stuff is done outside of LSM? Why isn't is part of the LSM so that say SELinux can have a syscall bitmask per security context? I could do that, but I like Will's approach better. From the PoV of meeting security goals of information flow, data confidentiality, integrity, least priv, etc limiting on the syscall boundary doesn't make a lot of sense. You just don't know enough there to enforce these things. These are the types of goals that SELinux and other LSMs have previously tried to enforce. From the PoV of making the kernel more resistant to attacks and making a process more resistant to misbehavior I think that the syscall boundary is appropriate. Although I could do it in SELinux it don't really want to do it there. In case people are interested or confused let me give my definition of two words I've used a bit in these conversations: discretionary and mandatory. Any time I talk about a 'discretionary' security decision it is a security decisions that a process imposed upon itself. Aka the choice to use seccomp is discretionary. The choice to mark our own file u-wx is discretionary. This isn't the best definition but it's one that works well in this discussion. Mandatory security is one enforce by a global policy. It's what selinux is all about. SELinux doesn't give hoot what a process wants to do, it enforces a global policy from the top down. You take over a process, well, too bad, you still have no choice but to follow the mandatory policy. The LSM does NOT enforce a mandatory access control model, it's just how it's been used in the past. Ingo appears to me (please correct me if I'm wrong) to really be a fan of exposing the flexibility of the LSM to a discretionary access control model. That doesn't seem like a bad idea. And maybe using the filter engine to define the language to do this isn't a bad idea either. But I think that's a 'down the road' project, not something to hold up a better seccomp. Making it part of the LSM also avoids having to add this prctl(). Well, it would mean exposing some new language construct to every LSM (instead of a single prctl construct) and it would mean anyone wanting to use the interface would have to rely on the LSM implementing those hooks the way they need it. Honestly chrome can already get all of the benefits of this patch (given a perfectly coded kernel) and a whole lot more using SELinux, but (surprise surprise) not everyone uses SELinux. I think it's a good idea to expose a simple interface which will be widely enough adopted that many userspace applications can rely on it for hardening. The existence of the LSM and the fact that there exists multiple security modules that may or may not be enabled really leads application developers to be unable to rely on LSM for security. If linux had a single security model which everyone could rely on we wouldn't really have as big of an issue but that's not possible. So I'm advocating for this series which will provide a single useful change which applications can rely upon across distros and platforms to enhance the properties and abilities of the linux kernel. -Eric ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
