Message ID | 20110603220451.23134.47368.stgit@paris.rdu.redhat.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Fri, Jun 3, 2011 at 3:04 PM, Eric Paris <eparis@redhat.com> wrote: > 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. v2 builds and boots on ia64 now Acked-by: Tony Luck <tony.luck@intel.com> > Signed-off-by: Eric Paris <eparis@redhat.com> > Acked-by: Acked-by: H. Peter Anvin <hpa@zytor.com> [for x86 portion] ^^^^^^^^^^^^^^^^^^^^ :-) -Tony -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Samstag 04 Juni 2011, 00:04:51 schrieb Eric Paris: > 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 <eparis@redhat.com> > Acked-by: Acked-by: H. Peter Anvin <hpa@zytor.com> [for x86 portion] The UML part is now fine for me. :-) Acked-by: Richard Weinberger <richard@nod.at> Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Paris <eparis@redhat.com> Date: Fri, 03 Jun 2011 18:04:51 -0400 ... > Signed-off-by: Eric Paris <eparis@redhat.com> > Acked-by: Acked-by: H. Peter Anvin <hpa@zytor.com> [for x86 portion] For sparc parts: Acked-by: David S. Miller <davem@davemloft.net> -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? Oleg. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 03, 2011 at 06:04:51PM -0400, Eric Paris wrote:
Thanks, this looks good & compiles, so:
Acked-by: Ralf Baechle <ralf@linux-mips.org>
I will rebase my pending MIPS audit patches on top of this patch and resend.
Thanks,
Ralf
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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... Oleg. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/08, Eric Paris wrote: > > On Wed, 2011-06-08 at 18:36 +0200, Oleg Nesterov wrote: > > 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. and probably by how much work is done in audit_syscall_entry/exit. OK. Thanks a lot Eric for your explanations. Oleg. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); - } + audit_syscall_exit(®s); 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..bec1c74 100644 --- a/arch/microblaze/include/asm/ptrace.h +++ b/arch/microblaze/include/asm/ptrace.h @@ -61,6 +61,11 @@ struct pt_regs { #define instruction_pointer(regs) ((regs)->pc) #define profile_pc(regs) instruction_pointer(regs) +static inline long regs_return_value(struct pt_regs *regs) +{ + return 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); + audit_syscall_exit(regs); step = test_thread_flag(TIF_SINGLESTEP); if (step || test_thread_flag(TIF_SYSCALL_TRACE)) diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h index de39b1f..7d40950 100644 --- a/arch/mips/include/asm/ptrace.h +++ b/arch/mips/include/asm/ptrace.h @@ -137,7 +137,19 @@ extern int ptrace_set_watch_regs(struct task_struct *child, */ #define user_mode(regs) (((regs)->cp0_status & KU_MASK) == KU_USER) -#define regs_return_value(_regs) ((_regs)->regs[2]) +static inline int is_syscall_success(struct pt_regs *regs) +{ + return !regs->regs[7]; +} + +static inline long regs_return_value(struct pt_regs *regs) +{ + if (is_syscall_success(regs)) + return regs->regs[2]; + else + return -regs->regs[2]; +} + #define instruction_pointer(regs) ((regs)->cp0_epc) #define profile_pc(regs) instruction_pointer(regs) diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c index 4e6ea1f..ab0f196 100644 --- a/arch/mips/kernel/ptrace.c +++ b/arch/mips/kernel/ptrace.c @@ -572,9 +572,7 @@ out: */ asmlinkage void syscall_trace_leave(struct pt_regs *regs) { - if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(regs->regs[7]), - -regs->regs[2]); + audit_syscall_exit(regs); if (!(current->ptrace & PT_PTRACED)) return; diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index 48223f9..679aa7b 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -86,7 +86,15 @@ struct pt_regs { #define instruction_pointer(regs) ((regs)->nip) #define user_stack_pointer(regs) ((regs)->gpr[1]) #define kernel_stack_pointer(regs) ((regs)->gpr[1]) -#define regs_return_value(regs) ((regs)->gpr[3]) +static inline int is_syscall_success(struct pt_regs *regs) +{ + return !(regs->ccr & 0x10000000); +} + +static inline long regs_return_value(struct pt_regs *regs) +{ + return regs->gpr[3]; +} #ifdef CONFIG_SMP extern unsigned long profile_pc(struct pt_regs *regs); diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index cb22024..d801ab7 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -1741,9 +1741,7 @@ void do_syscall_trace_leave(struct pt_regs *regs) { int step; - if (unlikely(current->audit_context)) - audit_syscall_exit((regs->ccr&0x10000000)?AUDITSC_FAILURE:AUDITSC_SUCCESS, - regs->result); + audit_syscall_exit(regs); if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->result); diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h index 9ad628a..df397e8 100644 --- a/arch/s390/include/asm/ptrace.h +++ b/arch/s390/include/asm/ptrace.h @@ -544,9 +544,13 @@ extern void show_regs(struct pt_regs * regs); #define user_mode(regs) (((regs)->psw.mask & PSW_MASK_PSTATE) != 0) #define instruction_pointer(regs) ((regs)->psw.addr & PSW_ADDR_INSN) #define user_stack_pointer(regs)((regs)->gprs[15]) -#define regs_return_value(regs)((regs)->gprs[2]) #define profile_pc(regs) instruction_pointer(regs) +static inline long regs_return_value(struct pt_regs *regs) +{ + return regs->gprs[2]; +} + int regs_query_register_offset(const char *name); const char *regs_query_register_name(unsigned int offset); unsigned long regs_get_register(struct pt_regs *regs, unsigned int offset); diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c index ef86ad2..d4d6fcb 100644 --- a/arch/s390/kernel/ptrace.c +++ b/arch/s390/kernel/ptrace.c @@ -753,9 +753,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) asmlinkage void do_syscall_trace_exit(struct pt_regs *regs) { - if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(regs->gprs[2]), - regs->gprs[2]); + audit_syscall_exit(regs); if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->gprs[2]); diff --git a/arch/sh/include/asm/ptrace_32.h b/arch/sh/include/asm/ptrace_32.h index 6c2239c..2d3e906 100644 --- a/arch/sh/include/asm/ptrace_32.h +++ b/arch/sh/include/asm/ptrace_32.h @@ -76,7 +76,10 @@ struct pt_dspregs { #ifdef __KERNEL__ #define MAX_REG_OFFSET offsetof(struct pt_regs, tra) -#define regs_return_value(_regs) ((_regs)->regs[0]) +static inline long regs_return_value(struct pt_regs *regs) +{ + return regs->regs[0]; +} #endif /* __KERNEL__ */ diff --git a/arch/sh/include/asm/ptrace_64.h b/arch/sh/include/asm/ptrace_64.h index bf9be77..eb3fcce 100644 --- a/arch/sh/include/asm/ptrace_64.h +++ b/arch/sh/include/asm/ptrace_64.h @@ -13,7 +13,10 @@ struct pt_regs { #ifdef __KERNEL__ #define MAX_REG_OFFSET offsetof(struct pt_regs, tregs[7]) -#define regs_return_value(_regs) ((_regs)->regs[3]) +static inline long regs_return_value(struct pt_regs *regs) +{ + return regs->regs[3]; +} #endif /* __KERNEL__ */ diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c index 3d7b209..5fce97e 100644 --- a/arch/sh/kernel/ptrace_32.c +++ b/arch/sh/kernel/ptrace_32.c @@ -529,9 +529,7 @@ asmlinkage void do_syscall_trace_leave(struct pt_regs *regs) { int step; - if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(regs->regs[0]), - regs->regs[0]); + audit_syscall_exit(regs); if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->regs[0]); diff --git a/arch/sh/kernel/ptrace_64.c b/arch/sh/kernel/ptrace_64.c index c8f9764..ba720d6 100644 --- a/arch/sh/kernel/ptrace_64.c +++ b/arch/sh/kernel/ptrace_64.c @@ -548,9 +548,7 @@ asmlinkage void do_syscall_trace_leave(struct pt_regs *regs) { int step; - if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(regs->regs[9]), - regs->regs[9]); + audit_syscall_exit(regs); if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->regs[9]); diff --git a/arch/sparc/include/asm/ptrace.h b/arch/sparc/include/asm/ptrace.h index c7ad3fe..10f98ca 100644 --- a/arch/sparc/include/asm/ptrace.h +++ b/arch/sparc/include/asm/ptrace.h @@ -206,7 +206,15 @@ do { current_thread_info()->syscall_noerror = 1; \ #define user_mode(regs) (!((regs)->tstate & TSTATE_PRIV)) #define instruction_pointer(regs) ((regs)->tpc) #define user_stack_pointer(regs) ((regs)->u_regs[UREG_FP]) -#define regs_return_value(regs) ((regs)->u_regs[UREG_I0]) +static inline int is_syscall_success(struct pt_regs *regs) +{ + return !(regs->tstate & (TSTATE_XCARRY | TSTATE_ICARRY)); +} + +static inline long regs_return_value(struct pt_regs *regs) +{ + return regs->u_regs[UREG_I0]; +} #ifdef CONFIG_SMP extern unsigned long profile_pc(struct pt_regs *); #else diff --git a/arch/sparc/kernel/ptrace_64.c b/arch/sparc/kernel/ptrace_64.c index 96ee50a..c73c8c5 100644 --- a/arch/sparc/kernel/ptrace_64.c +++ b/arch/sparc/kernel/ptrace_64.c @@ -1086,17 +1086,8 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs) asmlinkage void syscall_trace_leave(struct pt_regs *regs) { -#ifdef CONFIG_AUDITSYSCALL - if (unlikely(current->audit_context)) { - unsigned long tstate = regs->tstate; - int result = AUDITSC_SUCCESS; + audit_syscall_exit(regs); - if (unlikely(tstate & (TSTATE_XCARRY | TSTATE_ICARRY))) - result = AUDITSC_FAILURE; - - audit_syscall_exit(result, regs->u_regs[UREG_I0]); - } -#endif if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->u_regs[UREG_G1]); diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c index 701b672..839141d 100644 --- a/arch/um/kernel/ptrace.c +++ b/arch/um/kernel/ptrace.c @@ -203,8 +203,8 @@ void syscall_trace(struct uml_pt_regs *regs, int entryexit) UPT_SYSCALL_ARG2(regs), UPT_SYSCALL_ARG3(regs), UPT_SYSCALL_ARG4(regs)); - else audit_syscall_exit(AUDITSC_RESULT(UPT_SYSCALL_RET(regs)), - UPT_SYSCALL_RET(regs)); + else + audit_syscall_exit(regs); } /* Fake a debug trap */ diff --git a/arch/um/sys-i386/shared/sysdep/ptrace.h b/arch/um/sys-i386/shared/sysdep/ptrace.h index d50e62e..54ca089 100644 --- a/arch/um/sys-i386/shared/sysdep/ptrace.h +++ b/arch/um/sys-i386/shared/sysdep/ptrace.h @@ -162,6 +162,10 @@ 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) +static inline long regs_return_value(struct uml_pt_regs *regs) +{ + return UPT_SYSCALL_RET(regs); +} #define UPT_FAULTINFO(r) (&(r)->faultinfo) diff --git a/arch/um/sys-x86_64/shared/sysdep/ptrace.h b/arch/um/sys-x86_64/shared/sysdep/ptrace.h index fdba545..0924e20 100644 --- a/arch/um/sys-x86_64/shared/sysdep/ptrace.h +++ b/arch/um/sys-x86_64/shared/sysdep/ptrace.h @@ -124,6 +124,10 @@ struct uml_pt_regs { #define UPT_EFLAGS(r) REGS_EFLAGS((r)->gp) #define UPT_SYSCALL_NR(r) ((r)->syscall) #define UPT_SYSCALL_RET(r) UPT_RAX(r) +static inline long regs_return_value(struct uml_pt_regs *regs) +{ + return UPT_SYSCALL_RET(regs); +} extern int user_context(unsigned long sp); diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S index c1870dd..6d1572f 100644 --- a/arch/x86/ia32/ia32entry.S +++ b/arch/x86/ia32/ia32entry.S @@ -14,6 +14,7 @@ #include <asm/segment.h> #include <asm/irqflags.h> #include <linux/linkage.h> +#include <linux/err.h> /* Avoid __ASSEMBLER__'ifying <linux/audit.h> just for this. */ #include <linux/elf-em.h> @@ -210,13 +211,12 @@ sysexit_from_sys_call: TRACE_IRQS_ON sti movl %eax,%esi /* second arg, syscall return value */ - cmpl $0,%eax /* is it < 0? */ - setl %al /* 1 if so, 0 if not */ + cmpl $-MAX_ERRNO,%eax /* is it an error ? */ + 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 GET_THREAD_INFO(%r10) - movl RAX-ARGOFFSET(%rsp),%eax /* reload syscall return value */ + movq RAX-ARGOFFSET(%rsp),%rax /* reload syscall return value */ movl $(_TIF_ALLWORK_MASK & ~_TIF_SYSCALL_AUDIT),%edi cli TRACE_IRQS_OFF diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index 5c1a919..584112a 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -54,6 +54,7 @@ #include <asm/ftrace.h> #include <asm/irq_vectors.h> #include <asm/cpufeature.h> +#include <linux/err.h> /* Avoid __ASSEMBLER__'ifying <linux/audit.h> just for this. */ #include <linux/elf-em.h> @@ -465,11 +466,10 @@ sysexit_audit: TRACE_IRQS_ON ENABLE_INTERRUPTS(CLBR_ANY) movl %eax,%edx /* second arg, syscall return value */ - cmpl $0,%eax /* is it < 0? */ - setl %al /* 1 if so, 0 if not */ + cmpl $-MAX_ERRNO,%eax /* is it an error ? */ + setbe %al /* 1 if so, 0 if not */ movzbl %al,%eax /* zero-extend that */ - inc %eax /* first arg, 0->1(AUDITSC_SUCCESS), 1->2(AUDITSC_FAILURE) */ - call audit_syscall_exit + call __audit_syscall_exit DISABLE_INTERRUPTS(CLBR_ANY) TRACE_IRQS_OFF movl TI_flags(%ebp), %ecx 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 movl $(_TIF_ALLWORK_MASK & ~_TIF_SYSCALL_AUDIT),%edi jmp sysret_check #endif /* CONFIG_AUDITSYSCALL */ diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c index 807c2a2..5c6281e 100644 --- a/arch/x86/kernel/ptrace.c +++ b/arch/x86/kernel/ptrace.c @@ -1412,8 +1412,7 @@ void syscall_trace_leave(struct pt_regs *regs) { bool step; - if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(regs->ax), regs->ax); + audit_syscall_exit(regs); if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_exit(regs, regs->ax); diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c index 863f875..af17e1c 100644 --- a/arch/x86/kernel/vm86_32.c +++ b/arch/x86/kernel/vm86_32.c @@ -335,9 +335,9 @@ static void do_sys_vm86(struct kernel_vm86_struct *info, struct task_struct *tsk if (info->flags & VM86_SCREEN_BITMAP) mark_screen_rdonly(tsk->mm); - /*call audit_syscall_exit since we do not exit via the normal paths */ + /*call __audit_syscall_exit since we do not exit via the normal paths */ if (unlikely(current->audit_context)) - audit_syscall_exit(AUDITSC_RESULT(0), 0); + __audit_syscall_exit(1, 0); __asm__ __volatile__( "movl %0,%%esp\n\t" diff --git a/include/linux/audit.h b/include/linux/audit.h index 208efd6..0452dfc 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -26,6 +26,7 @@ #include <linux/types.h> #include <linux/elf-em.h> +#include <linux/ptrace.h> /* The netlink messages for the audit system is divided into blocks: * 1000 - 1099 are for commanding the audit system @@ -410,10 +411,6 @@ struct audit_field { void *lsm_rule; }; -#define AUDITSC_INVALID 0 -#define AUDITSC_SUCCESS 1 -#define AUDITSC_FAILURE 2 -#define AUDITSC_RESULT(x) ( ((long)(x))<0?AUDITSC_FAILURE:AUDITSC_SUCCESS ) extern int __init audit_register_class(int class, unsigned *list); extern int audit_classify_syscall(int abi, unsigned syscall); extern int audit_classify_arch(int arch); @@ -426,7 +423,7 @@ extern void audit_free(struct task_struct *task); extern void audit_syscall_entry(int arch, int major, unsigned long a0, unsigned long a1, unsigned long a2, unsigned long a3); -extern void audit_syscall_exit(int failed, long return_code); +extern void __audit_syscall_exit(int ret_success, long ret_value); extern void __audit_getname(const char *name); extern void audit_putname(const char *name); extern void __audit_inode(const char *name, const struct dentry *dentry); @@ -441,6 +438,15 @@ static inline int audit_dummy_context(void) void *p = current->audit_context; return !p || *(int *)p; } +static inline void audit_syscall_exit(void *pt_regs) +{ + if (unlikely(current->audit_context)) { + int success = is_syscall_success(pt_regs); + int return_code = regs_return_value(pt_regs); + + __audit_syscall_exit(success, return_code); + } +} static inline void audit_getname(const char *name) { if (unlikely(!audit_dummy_context())) @@ -560,12 +566,12 @@ static inline void audit_mmap_fd(int fd, int flags) extern int audit_n_rules; extern int audit_signals; -#else +#else /* CONFIG_AUDITSYSCALL */ #define audit_finish_fork(t) #define audit_alloc(t) ({ 0; }) #define audit_free(t) do { ; } while (0) #define audit_syscall_entry(ta,a,b,c,d,e) do { ; } while (0) -#define audit_syscall_exit(f,r) do { ; } while (0) +#define audit_syscall_exit(r) do { ; } while (0) #define audit_dummy_context() 1 #define audit_getname(n) do { ; } while (0) #define audit_putname(n) do { ; } while (0) @@ -597,7 +603,7 @@ extern int audit_signals; #define audit_rng(d, c, l) (0) #define audit_n_rules 0 #define audit_signals 0 -#endif +#endif /* CONFIG_AUDITSYSCALL */ #ifdef CONFIG_AUDIT /* These are defined in audit.c */ diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index 9178d5c..29bb846 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -98,6 +98,7 @@ #include <linux/compiler.h> /* For unlikely. */ #include <linux/sched.h> /* For struct task_struct. */ +#include <linux/err.h> /* for IS_ERR_VALUE */ extern long arch_ptrace(struct task_struct *child, long request, @@ -223,6 +224,15 @@ static inline void ptrace_release_task(struct task_struct *task) #define force_successful_syscall_return() do { } while (0) #endif +#ifndef is_syscall_success +/* + * On most systems we can tell if a syscall is a success based on if the retval + * is an error value. On some systems like ia64 and powerpc they have different + * indicators of success/failure and must define their own. + */ +#define is_syscall_success(regs) (!IS_ERR_VALUE((unsigned long)(regs_return_value(regs)))) +#endif + /* * <asm/ptrace.h> should define the following things inside #ifdef __KERNEL__. * diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 9c61afe..d21a34f 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -70,6 +70,11 @@ #include "audit.h" +/* flags stating the success for a syscall */ +#define AUDITSC_INVALID 0 +#define AUDITSC_SUCCESS 1 +#define AUDITSC_FAILURE 2 + /* AUDIT_NAMES is the number of slots we reserve in the audit_context * for saving names from getname(). If we get more names we will allocate * a name dynamically and also add those to the list anchored by names_list. */ @@ -1737,8 +1742,7 @@ void audit_finish_fork(struct task_struct *child) /** * audit_syscall_exit - deallocate audit context after a system call - * @valid: success/failure flag - * @return_code: syscall return value + * @pt_regs: syscall registers * * Tear down after system call. If the audit context has been marked as * auditable (either because of the AUDIT_RECORD_CONTEXT state from @@ -1746,13 +1750,17 @@ void audit_finish_fork(struct task_struct *child) * message), then write out the syscall information. In call cases, * free the names stored from getname(). */ -void audit_syscall_exit(int valid, long return_code) +void __audit_syscall_exit(int success, long return_code) { struct task_struct *tsk = current; struct audit_context *context; - context = audit_get_context(tsk, valid, return_code); + if (success) + success = AUDITSC_SUCCESS; + else + success = AUDITSC_FAILURE; + context = audit_get_context(tsk, success, return_code); if (likely(!context)) return;