[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [patch] fix syscall speedup patch mips typo



On Sat, 4 Mar 2006, Steve Grubb wrote:

> On Friday 03 March 2006 11:03, Jason Baron wrote:
> > I think this one is vary obvious. oops.
> 
> Ummm...your patch has never been posted to the linux-audit mail list yet. We 
> need to have the full patch posted so people can review it.
> 
> Thanks,
> -Steve
> 

Patch is below. The idea behind this patch is based on a suggestion from 
Steve Grubb to not call 'audit_syscall_entry' and 'audit_syscall_exit' if 
there are no audit rules loaded. This is problematic for the case where 
audit_log() is called in the middle of a system call (since we don't have 
the entry parameters). We address this issue by creating a partial system 
call record for this case, which contains the system call data that is 
available at exit time. The patch shows a 30% performance increase for the 
case where no rules are loaded on testing system calls in a tight loop.

thanks,

-Jason


diff-tree 7f3c303e61f8b545ccae8d71aff79a04f9d34c07 (from 86ce1392578d50fe9e46887de9c2dcba03c187e8)
tree e9b042fdd42b089d33be2ce33537c555403aef50
parent 86ce1392578d50fe9e46887de9c2dcba03c187e8
author Jason Baron <jbaron redhat com> 1141250157 -0500
committer Al Viro <viro zeniv linux org uk> 1141312115 -0500

    [PATCH] wrapping audit_syscall_exit()
    
    On Mon, 27 Feb 2006, Steve Grubb wrote:
    
    > On Friday 24 February 2006 18:29, Jason Baron wrote:
    > > that was to make the inlines in audit.h work. if you have another way to
    > > do it, i'd be more than happy to do it that way.
    >
    > Maybe we could put invoke_audit at the top of the structure and document in
    > the struct that it must be the first item or things break. Then you could do
    > something like:
    >
    > +static inline int invoke_audit_entry(void)
    > +{
    > +       int *invoke_audit = (int *)current->audit_context;
    > +       if (likely(!invoke_audit)
    > +               return 0;
    > +       return ( *invoke_audit = n_audit_rules );
    > +}
    >
    >
    > This will let the inlines work and keep the encapsulation of audit context to
    > auditsc.c. Albeit that we need documentation since its a gentleman's
    > agreement that the first entry in the structure has to be invoke_audit.
    >
    > -Steve
    >
    
    ok. the patch below is against audit-current and should address these
    concerns. I have not tested the audit partial path yet, though. thanks.
    
    [AV: fixed race in audit_add_rule]
    
    Signed-off-by: Al Viro <viro zeniv linux org uk>

diff --git a/arch/i386/kernel/ptrace.c b/arch/i386/kernel/ptrace.c
index 5c1fb6a..4caeee9 100644
--- a/arch/i386/kernel/ptrace.c
+++ b/arch/i386/kernel/ptrace.c
@@ -670,9 +670,11 @@ int do_syscall_trace(struct pt_regs *reg
 		secure_computing(regs->orig_eax);
 
 	if (unlikely(current->audit_context)) {
-		if (entryexit)
-			audit_syscall_exit(current, AUDITSC_RESULT(regs->eax),
-						regs->eax);
+		if (entryexit) {
+			if (audit_invoke_exit())
+				audit_syscall_exit(current, AUDITSC_RESULT(regs->eax),
+							regs->eax);
+		}
 		/* Debug traps, when using PTRACE_SINGLESTEP, must be sent only
 		 * on the syscall exit path. Normally, when TIF_SYSCALL_AUDIT is
 		 * not used, entry.S will call us only on syscall exit, not
@@ -719,14 +721,14 @@ int do_syscall_trace(struct pt_regs *reg
 	}
 	ret = is_sysemu;
 out:
-	if (unlikely(current->audit_context) && !entryexit)
+	if (audit_invoke_entry() && !entryexit)
 		audit_syscall_entry(current, AUDIT_ARCH_I386, regs->orig_eax,
-				    regs->ebx, regs->ecx, regs->edx, regs->esi);
+					regs->ebx, regs->ecx, regs->edx, regs->esi);
 	if (ret == 0)
 		return 0;
 
 	regs->orig_eax = -1; /* force skip of syscall restarting */
-	if (unlikely(current->audit_context))
+	if (audit_invoke_exit())
 		audit_syscall_exit(current, AUDITSC_RESULT(regs->eax),
 				regs->eax);
 	return 1;
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 9887c87..bc3fdc0 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1632,7 +1632,7 @@ syscall_trace_enter (long arg0, long arg
 	    && (current->ptrace & PT_PTRACED))
 		syscall_trace();
 
-	if (unlikely(current->audit_context)) {
+	if (audit_invoke_entry()) {
 		long syscall;
 		int arch;
 
@@ -1656,7 +1656,7 @@ syscall_trace_leave (long arg0, long arg
 		     long arg4, long arg5, long arg6, long arg7,
 		     struct pt_regs regs)
 {
-	if (unlikely(current->audit_context)) {
+	if (audit_invoke_exit()) {
 		int success = AUDITSC_RESULT(regs.r10);
 		long result = regs.r8;
 
diff --git a/arch/mips/kernel/ptrace.c b/arch/mips/kernel/ptrace.c
index f838b36..ddeec87 100644
--- a/arch/mips/kernel/ptrace.c
+++ b/arch/mips/kernel/ptrace.c
@@ -468,7 +468,7 @@ static inline int audit_arch(void)
  */
 asmlinkage void do_syscall_trace(struct pt_regs *regs, int entryexit)
 {
-	if (unlikely(current->audit_context) && entryexit)
+	if (audit_invoke_exit && entryexit)
 		audit_syscall_exit(current, AUDITSC_RESULT(regs->regs[2]),
 		                   regs->regs[2]);
 
@@ -492,7 +492,7 @@ asmlinkage void do_syscall_trace(struct 
 		current->exit_code = 0;
 	}
  out:
-	if (unlikely(current->audit_context) && !entryexit)
+	if (audit_invoke_entry && !entryexit)
 		audit_syscall_entry(current, audit_arch(), regs->regs[2],
 				    regs->regs[4], regs->regs[5],
 				    regs->regs[6], regs->regs[7]);
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 400793c..068d114 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -537,16 +537,16 @@ void do_syscall_trace_enter(struct pt_re
 	    && (current->ptrace & PT_PTRACED))
 		do_syscall_trace();
 
-	if (unlikely(current->audit_context))
+	if (audit_invoke_entry())
 		audit_syscall_entry(current,
 #ifdef CONFIG_PPC32
-				    AUDIT_ARCH_PPC,
+		AUDIT_ARCH_PPC,
 #else
-				    test_thread_flag(TIF_32BIT)?AUDIT_ARCH_PPC:AUDIT_ARCH_PPC64,
+		test_thread_flag(TIF_32BIT)?AUDIT_ARCH_PPC:AUDIT_ARCH_PPC64,
 #endif
-				    regs->gpr[0],
-				    regs->gpr[3], regs->gpr[4],
-				    regs->gpr[5], regs->gpr[6]);
+		regs->gpr[0],
+		regs->gpr[3], regs->gpr[4],
+		regs->gpr[5], regs->gpr[6]);
 }
 
 void do_syscall_trace_leave(struct pt_regs *regs)
@@ -555,10 +555,10 @@ void do_syscall_trace_leave(struct pt_re
 	secure_computing(regs->gpr[0]);
 #endif
 
-	if (unlikely(current->audit_context))
+	if (audit_invoke_exit())
 		audit_syscall_exit(current,
-				   (regs->ccr&0x1000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
-				   regs->result);
+				(regs->ccr&0x1000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
+				regs->result);
 
 	if ((test_thread_flag(TIF_SYSCALL_TRACE)
 #ifdef CONFIG_PPC64
diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 37dfe33..a7e94f4 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -733,7 +733,7 @@ out:
 asmlinkage void
 syscall_trace(struct pt_regs *regs, int entryexit)
 {
-	if (unlikely(current->audit_context) && entryexit)
+	if (audit_invoke_exit() && entryexit)
 		audit_syscall_exit(current, AUDITSC_RESULT(regs->gprs[2]), regs->gprs[2]);
 
 	if (!test_thread_flag(TIF_SYSCALL_TRACE))
@@ -760,7 +760,7 @@ syscall_trace(struct pt_regs *regs, int 
 		current->exit_code = 0;
 	}
  out:
-	if (unlikely(current->audit_context) && !entryexit)
+	if (audit_invoke_entry() && !entryexit)
 		audit_syscall_entry(current, 
 				    test_thread_flag(TIF_31BIT)?AUDIT_ARCH_S390:AUDIT_ARCH_S390X,
 				    regs->gprs[2], regs->orig_gpr2, regs->gprs[3],
diff --git a/arch/sparc64/kernel/ptrace.c b/arch/sparc64/kernel/ptrace.c
index 3f9746f..5516f2c 100644
--- a/arch/sparc64/kernel/ptrace.c
+++ b/arch/sparc64/kernel/ptrace.c
@@ -620,7 +620,7 @@ asmlinkage void syscall_trace(struct pt_
 	/* do the secure computing check first */
 	secure_computing(regs->u_regs[UREG_G1]);
 
-	if (unlikely(current->audit_context) && syscall_exit_p) {
+	if (audit_invoke_exit() && syscall_exit_p) {
 		unsigned long tstate = regs->tstate;
 		int result = AUDITSC_SUCCESS;
 
@@ -650,7 +650,7 @@ asmlinkage void syscall_trace(struct pt_
 	}
 
 out:
-	if (unlikely(current->audit_context) && !syscall_exit_p)
+	if (audit_invoke_entry() && !syscall_exit_p)
 		audit_syscall_entry(current,
 				    (test_thread_flag(TIF_32BIT) ?
 				     AUDIT_ARCH_SPARC :
diff --git a/arch/um/kernel/ptrace.c b/arch/um/kernel/ptrace.c
index 98e0939..26dde51 100644
--- a/arch/um/kernel/ptrace.c
+++ b/arch/um/kernel/ptrace.c
@@ -267,19 +267,19 @@ void syscall_trace(union uml_pt_regs *re
 	int is_singlestep = (current->ptrace & PT_DTRACE) && entryexit;
 	int tracesysgood;
 
-	if (unlikely(current->audit_context)) {
-		if (!entryexit)
+	if (!entryexit)
+		if(audit_invoke_entry())
 			audit_syscall_entry(current,
-                                            HOST_AUDIT_ARCH,
-					    UPT_SYSCALL_NR(regs),
-					    UPT_SYSCALL_ARG1(regs),
-					    UPT_SYSCALL_ARG2(regs),
-					    UPT_SYSCALL_ARG3(regs),
-					    UPT_SYSCALL_ARG4(regs));
-		else audit_syscall_exit(current,
-                                        AUDITSC_RESULT(UPT_SYSCALL_RET(regs)),
-                                        UPT_SYSCALL_RET(regs));
-	}
+					HOST_AUDIT_ARCH,
+					UPT_SYSCALL_NR(regs),
+					UPT_SYSCALL_ARG1(regs),
+					UPT_SYSCALL_ARG2(regs),
+					UPT_SYSCALL_ARG3(regs),
+					UPT_SYSCALL_ARG4(regs));
+	else if (audit_invoke_exit())
+		audit_syscall_exit(current,
+				AUDITSC_RESULT(UPT_SYSCALL_RET(regs)),
+				UPT_SYSCALL_RET(regs));
 
 	/* Fake a debug trap */
 	if (is_singlestep)
diff --git a/arch/x86_64/kernel/ptrace.c b/arch/x86_64/kernel/ptrace.c
index 5320562..c669c78 100644
--- a/arch/x86_64/kernel/ptrace.c
+++ b/arch/x86_64/kernel/ptrace.c
@@ -603,24 +603,24 @@ asmlinkage void syscall_trace_enter(stru
 	    && (current->ptrace & PT_PTRACED))
 		syscall_trace(regs);
 
-	if (unlikely(current->audit_context)) {
+	if (audit_invoke_entry()) {
 		if (test_thread_flag(TIF_IA32)) {
 			audit_syscall_entry(current, AUDIT_ARCH_I386,
-					    regs->orig_rax,
-					    regs->rbx, regs->rcx,
-					    regs->rdx, regs->rsi);
+						regs->orig_rax,
+						regs->rbx, regs->rcx,
+						regs->rdx, regs->rsi);
 		} else {
 			audit_syscall_entry(current, AUDIT_ARCH_X86_64,
-					    regs->orig_rax,
-					    regs->rdi, regs->rsi,
-					    regs->rdx, regs->r10);
+						regs->orig_rax,
+						regs->rdi, regs->rsi,
+						regs->rdx, regs->r10);
 		}
 	}
 }
 
 asmlinkage void syscall_trace_leave(struct pt_regs *regs)
 {
-	if (unlikely(current->audit_context))
+	if (audit_invoke_exit())
 		audit_syscall_exit(current, AUDITSC_RESULT(regs->rax), regs->rax);
 
 	if ((test_thread_flag(TIF_SYSCALL_TRACE)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 1c47c59..5443dbb 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -83,6 +83,7 @@
 #define AUDIT_CONFIG_CHANGE	1305	/* Audit system configuration change */
 #define AUDIT_SOCKADDR		1306	/* sockaddr copied as syscall arg */
 #define AUDIT_CWD		1307	/* Current working directory */
+#define AUDIT_SYSCALL_PARTIAL  1310    /* Partial syscall event */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
@@ -320,6 +321,20 @@ extern int audit_sockaddr(int len, void 
 extern int audit_avc_path(struct dentry *dentry, struct vfsmount *mnt);
 extern void audit_signal_info(int sig, struct task_struct *t);
 extern int audit_set_macxattr(const char *name);
+extern int audit_n_rules;
+static inline int audit_invoke_entry(void)
+{
+	if (likely(!current->audit_context))
+		return 0;
+	return (*((int *)current->audit_context) = audit_n_rules);
+}
+
+static inline int audit_invoke_exit(void)
+{
+	if (likely(!current->audit_context))
+		return 0;
+	return (*(int *)current->audit_context);
+}
 #else
 #define audit_alloc(t) ({ 0; })
 #define audit_free(t) do { ; } while (0)
@@ -339,6 +354,9 @@ extern int audit_set_macxattr(const char
 #define audit_avc_path(dentry, mnt) ({ 0; })
 #define audit_signal_info(s,t) do { ; } while (0)
 #define audit_set_macxattr(n) do { ; } while (0)
+#define audit_n_rules 0
+#define audit_invoke_entry() ({ 0; })
+#define audit_invoke_exit() ({ 0; })
 #endif
 
 #ifdef CONFIG_AUDIT
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index fbdd19c..aeaba84 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -334,6 +334,7 @@ static inline int audit_add_rule(struct 
 				  struct list_head *list)
 {
 	struct audit_entry *e;
+	int dont_count = 0;
 
 	/* Do not use the _rcu iterator here, since this is the only
 	 * addition routine. */
@@ -342,11 +343,17 @@ static inline int audit_add_rule(struct 
 			return -EEXIST;
 	}
 
+	/* If either of these, don't count towards total */
+	if (entry->rule.listnr == AUDIT_FILTER_USER ||
+		entry->rule.listnr == AUDIT_FILTER_TYPE)
+		dont_count = 1;
 	if (entry->rule.flags & AUDIT_FILTER_PREPEND) {
 		list_add_rcu(&entry->list, list);
 	} else {
 		list_add_tail_rcu(&entry->list, list);
 	}
+	if (!dont_count)
+		audit_n_rules++;
 
 	return 0;
 }
@@ -363,7 +370,11 @@ static inline int audit_del_rule(struct 
 	list_for_each_entry(e, list, list) {
 		if (!audit_compare_rule(&entry->rule, &e->rule)) {
 			list_del_rcu(&e->list);
+			if (entry->rule.listnr == AUDIT_FILTER_USER ||
+				entry->rule.listnr == AUDIT_FILTER_TYPE)
+				audit_n_rules++;	
 			call_rcu(&e->rcu, audit_free_rule_rcu);
+			audit_n_rules--;
 			return 0;
 		}
 	}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index c317ef4..efbcae5 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -75,6 +75,9 @@ extern int audit_enabled;
  * path_lookup. */
 #define AUDIT_NAMES_RESERVED 7
 
+/* number of audit rules */
+int audit_n_rules;
+
 /* When fs/namei.c:getname() is called, we store the pointer in name and
  * we don't let putname() free it (instead we free all of the saved
  * pointers at syscall exit time).
@@ -129,6 +132,9 @@ struct audit_aux_data_path {
 
 /* The per-task audit context. */
 struct audit_context {
+	int		    invoke_audit; /* 1 if we should do syscall auditing, important:
+						implementation relies on this item being the
+						first one in the data structure */	
 	int		    in_syscall;	/* 1 if task is in a syscall */
 	enum audit_state    state;
 	unsigned int	    serial;     /* serial number for record */
@@ -576,11 +582,14 @@ static void audit_log_exit(struct audit_
 	struct audit_aux_data *aux;
 	const char *tty;
 
-	ab = audit_log_start(context, gfp_mask, AUDIT_SYSCALL);
+	ab = audit_log_start(context, gfp_mask,
+			context->in_syscall ? AUDIT_SYSCALL: AUDIT_SYSCALL_PARTIAL);
 	if (!ab)
 		return;		/* audit_panic has been called */
-	audit_log_format(ab, "arch=%x syscall=%d",
-			 context->arch, context->major);
+	if (context->in_syscall) {
+		audit_log_format(ab, "arch=%x syscall=%d",
+			context->arch, context->major);
+	}
 	if (context->personality != PER_LINUX)
 		audit_log_format(ab, " per=%lx", context->personality);
 	if (context->return_valid)
@@ -591,22 +600,38 @@ static void audit_log_exit(struct audit_
 		tty = current->signal->tty->name;
 	else
 		tty = "(none)";
-	audit_log_format(ab,
-		  " a0=%lx a1=%lx a2=%lx a3=%lx items=%d"
-		  " pid=%d auid=%u uid=%u gid=%u"
-		  " euid=%u suid=%u fsuid=%u"
-		  " egid=%u sgid=%u fsgid=%u tty=%s",
-		  context->argv[0],
-		  context->argv[1],
-		  context->argv[2],
-		  context->argv[3],
-		  context->name_count,
-		  context->pid,
-		  context->loginuid,
-		  context->uid,
-		  context->gid,
-		  context->euid, context->suid, context->fsuid,
-		  context->egid, context->sgid, context->fsgid, tty);
+	if (context->in_syscall) {
+		audit_log_format(ab,
+			  " a0=%lx a1=%lx a2=%lx a3=%lx items=%d"
+			  " pid=%d auid=%u uid=%u gid=%u"
+		 	 " euid=%u suid=%u fsuid=%u"
+			  " egid=%u sgid=%u fsgid=%u tty=%s",
+		 	 context->argv[0],
+			  context->argv[1],
+			  context->argv[2],
+		 	 context->argv[3],
+		  	context->name_count,
+		  	context->pid,
+		  	context->loginuid,
+		  	context->uid,
+		  	context->gid,
+		  	context->euid, context->suid, context->fsuid,
+		  	context->egid, context->sgid, context->fsgid, tty);
+	} else {
+		audit_log_format(ab,
+			" items=%d"
+			" pid=%d auid=%u uid=%u gid=%u"
+			" euid=%u suid=%u fsuid=%u"
+			" egid=%u sgid=%u fsgid=%u tty=%s",
+			context->name_count,
+			context->pid,
+			context->loginuid,
+			context->uid,
+			context->gid,
+			context->euid, context->suid, context->fsuid,
+			context->egid, context->sgid, context->fsgid, tty);
+	}
+
 	audit_log_task_info(ab, gfp_mask);
 	audit_log_end(ab);
 
@@ -834,7 +859,7 @@ void audit_syscall_exit(struct task_stru
 	if (likely(!context))
 		goto out;
 
-	if (context->in_syscall && context->auditable)
+	if (context->auditable)
 		audit_log_exit(context, GFP_KERNEL);
 
 	context->in_syscall = 0;
@@ -1110,6 +1135,7 @@ void auditsc_get_stamp(struct audit_cont
 	t->tv_nsec = ctx->ctime.tv_nsec;
 	*serial    = ctx->serial;
 	ctx->auditable = 1;
+	ctx->invoke_audit = 1;
 }
 
 /**


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]