{PATCH] Audit Filter Performance

{PATCH] Audit Filter Performance

[Steve Grubb]

Hi,

While testing the watch performance, I noticed that selinux_task_ctxid() was
creeping into the results more than it should. Investigation showed that the
function call was being called whether it was needed or not. The below patch
fixes this.

Signed-off-by: Steve Grubb


diff -ur linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c	2006-04-08 16:28:16.000000000 -0400
+++ linux-2.6.16.x86_64/kernel/auditsc.c	2006-04-08 16:33:33.000000000 -0400
@@ -190,9 +190,6 @@
 			      enum audit_state *state)
 {
 	int i, j;
-	u32 sid;
-
-	selinux_task_ctxid(tsk, &sid);
 
 	for (i = 0; i < rule->field_count; i++) {
 		struct audit_field *f = &rule->fields[i];
@@ -295,11 +292,15 @@
 			   match for now to avoid losing information that
 			   may be wanted.   An error message will also be
 			   logged upon error */
-			if (f->se_rule)
+			if (f->se_rule) {
+				u32 sid;
+
+				selinux_task_ctxid(tsk, &sid);
 				result = selinux_audit_rule_match(sid, f->type,
 				                                  f->op,
 				                                  f->se_rule,
 				                                  ctx);
+			}
 			break;
 		case AUDIT_ARG0:
 		case AUDIT_ARG1:

Re: {PATCH] Audit Filter Performance

[Amy Griffis]

Steve Grubb wrote:  [Sat Apr 08 2006, 04:46:26PM EDT]
> While testing the watch performance, I noticed that selinux_task_ctxid() was
> creeping into the results more than it should. Investigation showed that the
> function call was being called whether it was needed or not. The below patch
> fixes this.

You've moved selinux_task_ctxid() inside a for loop.  Now it will be
called for each selinux field in a rule.  I don't think that's what
you want.  

A better solution would be to set a rule flag in
audit_data_to_entry(), then check that flag outside the for loop.

>
> 
> Signed-off-by: Steve Grubb
> 
> 
> diff -ur linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
> --- linux-2.6.16.x86_64.orig/kernel/auditsc.c	2006-04-08 16:28:16.000000000 -0400
> +++ linux-2.6.16.x86_64/kernel/auditsc.c	2006-04-08 16:33:33.000000000 -0400
> @@ -190,9 +190,6 @@
>  			      enum audit_state *state)
>  {
>  	int i, j;
> -	u32 sid;
> -
> -	selinux_task_ctxid(tsk, &sid);
>  
>  	for (i = 0; i < rule->field_count; i++) {
>  		struct audit_field *f = &rule->fields[i];
> @@ -295,11 +292,15 @@
>  			   match for now to avoid losing information that
>  			   may be wanted.   An error message will also be
>  			   logged upon error */
> -			if (f->se_rule)
> +			if (f->se_rule) {
> +				u32 sid;
> +
> +				selinux_task_ctxid(tsk, &sid);
>  				result = selinux_audit_rule_match(sid, f->type,
>  				                                  f->op,
>  				                                  f->se_rule,
>  				                                  ctx);
> +			}
>  			break;
>  		case AUDIT_ARG0:
>  		case AUDIT_ARG1:
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
> 

Re: {PATCH] Audit Filter Performance

[Steve Grubb]

On Monday 10 April 2006 19:46, Amy Griffis wrote:
> > While testing the watch performance, I noticed that selinux_task_ctxid()
> > was creeping into the results more than it should. Investigation showed
> > that the function call was being called whether it was needed or not. The
> > below patch fixes this.
>
> You've moved selinux_task_ctxid() inside a for loop.  Now it will be
> called for each selinux field in a rule.  I don't think that's what
> you want.  
>
> A better solution would be to set a rule flag in
> audit_data_to_entry(), then check that flag outside the for loop.

Yes, you are right - Thanks!  New patch below.

Signed-off-by: Steve Grubb <sgrubb@redhat.com>


diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
--- linux-2.6.16.x86_64.orig/kernel/auditsc.c	2006-04-11 08:44:02.000000000 -0400
+++ linux-2.6.16.x86_64/kernel/auditsc.c	2006-04-11 08:43:17.000000000 -0400
@@ -189,11 +189,9 @@ static int audit_filter_rules(struct tas
 			      struct audit_context *ctx,
 			      enum audit_state *state)
 {
-	int i, j;
+	int i, j, need_sid = 1;
 	u32 sid;
 
-	selinux_task_ctxid(tsk, &sid);
-
 	for (i = 0; i < rule->field_count; i++) {
 		struct audit_field *f = &rule->fields[i];
 		int result = 0;
@@ -295,11 +293,16 @@ static int audit_filter_rules(struct tas
 			   match for now to avoid losing information that
 			   may be wanted.   An error message will also be
 			   logged upon error */
-			if (f->se_rule)
+			if (f->se_rule) {
+				if (need_sid) {
+					selinux_task_ctxid(tsk, &sid);
+					need_sid = 0;
+				}
 				result = selinux_audit_rule_match(sid, f->type,
 				                                  f->op,
 				                                  f->se_rule,
 				                                  ctx);
+			}
 			break;
 		case AUDIT_ARG0:
 		case AUDIT_ARG1: