{PATCH] Audit Filter Performance

PATCH Steve Grubb 2 participants 3 messages Apr 8, 2006 – Apr 11, 2006 Audit

Share & tools

Steve Grubb

author

2006-04-08 20:46 UTC #0 lore raw

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

kernel/auditsc.c +5 -4

diff -ur linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
@@ -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:

Amy Griffis

2006-04-10 23:46 UTC #1 lore raw

Steve Grubb wrote: [Sat Apr 08 2006, 04:46:26PM EDT]

Quoted 4 lines

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.

Quoted 41 lines

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

kernel/auditsc.c +5 -5

@@ -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
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Steve Grubb

author

2006-04-11 12:50 UTC #2 lore raw

Quoted 11 lines · Amy Griffis

Quoted 4 lines
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 <[email protected]>

kernel/auditsc.c +7 -4

diff -urp linux-2.6.16.x86_64.orig/kernel/auditsc.c linux-2.6.16.x86_64/kernel/auditsc.c
@@ -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: