audit: speedup for syscalls when auditing is disabled

We found that when auditing is disabled using "auditctl -D", that there's still a significant overhead when doing syscalls. This overhead is not present when a single never rule is inserted using "auditctl -a task,never".

Using Anton's null syscall microbenchmark from
http://ozlabs.org/~anton/junkcode/null_syscall.c we currently have on a powerpc machine:

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     739.03 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

This doesn't seem right, as we'd hope that auditing would have the same minimal impact when disabled via -D as when we have a single never rule.

The patch below creates a fast path when initialising a task. If the rules list for tasks is empty (the disabled -D option), we mark auditing as disabled for this task.

When this is applied, our null syscall benchmark improves in the disabled case to match the single never rule case.

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     204.62 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

Reported-by Anton Blanchard <[email protected]>

Signed-off-by Michael Neuling <[email protected]>

---
I'm not familiar with the auditing code/infrastructure so I may have misunderstood something here

kernel/auditsc.c +0 -0

index 1b31c13..1cd6ec7 100644
@@ -666,6 +666,11 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
 	enum audit_state   state;

Quoted 41 lines · Michael Neuling

We found that when auditing is disabled using "auditctl -D", that there's still a significant overhead when doing syscalls. This overhead is not present when a single never rule is inserted using "auditctl -a task,never".

Using Anton's null syscall microbenchmark from
http://ozlabs.org/~anton/junkcode/null_syscall.c we currently have on a powerpc machine:

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     739.03 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

This doesn't seem right, as we'd hope that auditing would have the same minimal impact when disabled via -D as when we have a single never rule.

The patch below creates a fast path when initialising a task. If the rules list for tasks is empty (the disabled -D option), we mark auditing as disabled for this task.

When this is applied, our null syscall benchmark improves in the disabled case to match the single never rule case.

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     204.62 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

Reported-by Anton Blanchard <[email protected]>

Signed-off-by Michael Neuling <[email protected]>

---
I'm not familiar with the auditing code/infrastructure so I may have misunderstood something here

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_

Quoted 5 lines

kernel/auditsc.c +0 -0

index 1b31c13..1cd6ec7 100644
@@ -666,6 +666,11 @@ static enum audit_state audit_filter_task(struct task_

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)

Quoted 9 lines

kernel/auditsc.c +5 -0

 	enum audit_state   state;
 
 	rcu_read_lock();
+	/* Fast path.  If the list is empty, disable auditing */
+	if (list_empty(&audit_filter_list[AUDIT_FILTER_TASK])) {
+		rcu_read_unlock();
+		return AUDIT_DISABLED;
+	}
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list)

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_struct *tsk, char **key) · line 676

Quoted 2 lines

kernel/auditsc.c +0 -0

 		if (audit_filter_rules(tsk, &e->rule, NULL, NULL, &state)) {
 			if (state == AUDIT_RECORD_CONTEXT)

I don't think this works at all. I don't see how syscall audit'ing can work. What if I have nothing in the AUDIT_FILTER_TASK list but I want to audit all 'open(2)' syscalls? This patch is going to leave the task in the DISABLED state and we won't ever be able to match on the syscall rules.

I wonder if you could get much back, in terms of performance, by moving the

         context->dummy = !audit_n_rules;

line to the top and just returning if context->dummy == 1;

I'll play a bit, but I thought that was supposed to be a safe thing to do....

Quoted 42 lines · In message <[email protected]> you wrote:

Quoted 41 lines · Michael Neuling

We found that when auditing is disabled using "auditctl -D", that there's still a significant overhead when doing syscalls. This overhead is not present when a single never rule is inserted using "auditctl -a task,never".

Using Anton's null syscall microbenchmark from
http://ozlabs.org/~anton/junkcode/null_syscall.c we currently have on a powerpc machine:

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     739.03 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

This doesn't seem right, as we'd hope that auditing would have the same minimal impact when disabled via -D as when we have a single never rule.

The patch below creates a fast path when initialising a task. If the rules list for tasks is empty (the disabled -D option), we mark auditing as disabled for this task.

When this is applied, our null syscall benchmark improves in the disabled case to match the single never rule case.

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     204.62 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

Reported-by Anton Blanchard <[email protected]>

Signed-off-by Michael Neuling <[email protected]>

---
I'm not familiar with the auditing code/infrastructure so I may have misunderstood something here

Quoted 5 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_

Quoted 5 lines

kernel/auditsc.c +0 -0

index 1b31c13..1cd6ec7 100644
@@ -666,6 +666,11 @@ static enum audit_state audit_filter_task(struct task_

struct *tsk, char **key)

Quoted 9 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)

Quoted 9 lines

kernel/auditsc.c +5 -0

 	enum audit_state   state;
 
 	rcu_read_lock();
+	/* Fast path.  If the list is empty, disable auditing */
+	if (list_empty(&audit_filter_list[AUDIT_FILTER_TASK])) {
+		rcu_read_unlock();
+		return AUDIT_DISABLED;
+	}
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list)

{

Quoted 8 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct task_struct *tsk, char **key) · line 676

Quoted 2 lines

kernel/auditsc.c +0 -0

 		if (audit_filter_rules(tsk, &e->rule, NULL, NULL, &state)) {
 			if (state == AUDIT_RECORD_CONTEXT)

I don't think this works at all. I don't see how syscall audit'ing can work. What if I have nothing in the AUDIT_FILTER_TASK list but I want to audit all 'open(2)' syscalls? This patch is going to leave the task in the DISABLED state and we won't ever be able to match on the syscall rules.

Sorry my bad. I'm not too familiar with the audit infrastructure.

On reflection, we might have a bug in audit_alloc though. Currently we have this:

int audit_alloc(struct task_struct *tsk)

  {
	  <snip>
	  state = audit_filter_task(tsk, &key);
	  if (likely(state == AUDIT_DISABLED))
		  return 0;

	  <snip>
	  set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
	  return 0;

}

This gets called on fork. If we have "task,never" rule, we hit this state == AUDIT_DISABLED path, return immediately and the tasks TIF_SYSCALL_AUDIT flags doesn't get set. On powerpc, we check TIF_SYSCALL_AUDIT in asm on syscall entry to fast path not calling the syscall audit code.

This seems wrong to me as a "never" _task_ audit rule shouldn't effect _syscall_ auditing? Is there some interaction between task and syscall auditing that I'm missing?

Quoted 4 lines

I wonder if you could get much back, in terms of performance, by moving the

         context->dummy = !audit_n_rules;

line to the top and just returning if context->dummy == 1;

We get 668.09 cycles with this optimisation, so it comes down a bit, but no where near if the auditing is disabled altogether.

Like I said above, powerpc has a fast path in asm on system call entry to check the thread_info flags for if syscall auditing is disabled. If it's disabled, we don't call the audit code, hence why it's very fast in this case.

Quoted 7 lines

Quoted 4 lines

I wonder if you could get much back, in terms of performance, by moving the

         context->dummy = !audit_n_rules;

line to the top and just returning if context->dummy == 1;

We get 668.09 cycles with this optimisation, so it comes down a bit, but no where near if the auditing is disabled altogether.

Clean that patch up and send it. Sounds like a win no matter what else we do.

Quoted 15 lines

On reflection, we might have a bug in audit_alloc though. Currently we have this:

int audit_alloc(struct task_struct *tsk)

  {
	  <snip>
	  state = audit_filter_task(tsk, &key);
	  if (likely(state == AUDIT_DISABLED))
		  return 0;

	  <snip>
	  set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
	  return 0;

}

Quoted 5 lines

This gets called on fork. If we have "task,never" rule, we hit this state == AUDIT_DISABLED path, return immediately and the tasks TIF_SYSCALL_AUDIT flags doesn't get set. On powerpc, we check TIF_SYSCALL_AUDIT in asm on syscall entry to fast path not calling the syscall audit code.

I'm guessing it actually bypasses audit if the flag is not set?

So we might have a bug, but i'd be surprised since I think we tested audit on powerpc....

Quoted 3 lines

This seems wrong to me as a "never" _task_ audit rule shouldn't effect _syscall_ auditing? Is there some interaction between task and syscall auditing that I'm missing?

There are 3 states for a given task, I don't remember the names off the top of my head, so I'll guess with: on, off, build. 'Build' is the state most processes usually live in. In this state we collect audit information about the task during the whole syscall and then we might (likely) throw that information away at syscall exit.

Some types of audit rule, which alter this state, can be checked at either 'entry' or 'exit' (first rule wins) At syscall entry we only have enough information (questionable if we even have enough information at all but that's a different question) to filter based on the task. You can create rules that will audit all tasks, or in your case will explicitly disable auditing for all tasks.

Normally a process would be in the default 'build' state after syscall entry, we will collect information about the syscall, and then we will check syscall rules at exit. Once you explicitly say 'I do not want any audit messages for this task' you are in 'off' instead of 'build.'

Quoted 7 lines

Quoted 4 lines

I wonder if you could get much back, in terms of performance, by moving the

         context->dummy = !audit_n_rules;

line to the top and just returning if context->dummy == 1;

We get 668.09 cycles with this optimisation, so it comes down a bit, but no where near if the auditing is disabled altogether.

Clean that patch up and send it. Sounds like a win no matter what else we do.

Quoted 4 lines

Like I said above, powerpc has a fast path in asm on system call entry to check the thread_info flags for if syscall auditing is disabled. If it's disabled, we don't call the audit code, hence why it's very fast in this case.

Here's a new idea to think about with obvious tradeoffs. What do you think about doing a little bit of assembly rejiggering?

Add a new spot in the assembly which will call a function which will check if audit_n_rules > 0 and if so will set TIF_SYSCALL_AUDIT and if not will clear TIF_SYSCALL_AUDIT? It might make things slightly worse on systems which explictly disable audit and the flag would always be clear on every task (like you did with the explicit rule) but I'm guessing might be a win on systems with no rules which are wasting time on the audit slow path.....

Quoted 15 lines

Quoted 15 lines

On reflection, we might have a bug in audit_alloc though. Currently we have this:

int audit_alloc(struct task_struct *tsk)

  {
	  <snip>
	  state = audit_filter_task(tsk, &key);
	  if (likely(state == AUDIT_DISABLED))
		  return 0;

	  <snip>
	  set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
	  return 0;

}

Quoted 7 lines

Quoted 5 lines

This gets called on fork. If we have "task,never" rule, we hit this state == AUDIT_DISABLED path, return immediately and the tasks TIF_SYSCALL_AUDIT flags doesn't get set. On powerpc, we check TIF_SYSCALL_AUDIT in asm on syscall entry to fast path not calling the syscall audit code.

I'm guessing it actually bypasses audit if the flag is not set?

Correct.

Quoted 2 lines

So we might have a bug, but i'd be surprised since I think we tested audit on powerpc....

So on powerpc we have this in entry_64.S

#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)

	andi.	r11,r10,_TIF_SYSCALL_T_OR_A
	bne-	syscall_dotrace

and there is similar code in x86 in entry_64.S

#ifdef CONFIG_AUDITSYSCALL
bt $TIF_SYSCALL_AUDIT,%edx
jc sysret_audit
#endif

So I think other archs are broken too.

I think the bug is in the generic code though. Shouldn't syscall auditing be enabled independent of task fork/exec auditing?

Quoted 10 lines

Quoted 3 lines

This seems wrong to me as a "never" _task_ audit rule shouldn't effect _syscall_ auditing? Is there some interaction between task and syscall auditing that I'm missing?

There are 3 states for a given task, I don't remember the names off the top of my head, so I'll guess with: on, off, build. 'Build' is the state most processes usually live in. In this state we collect audit information about the task during the whole syscall and then we might (likely) throw that information away at syscall exit.

Quoted 6 lines

Some types of audit rule, which alter this state, can be checked at either 'entry' or 'exit' (first rule wins) At syscall entry we only have enough information (questionable if we even have enough information at all but that's a different question) to filter based on the task. You can create rules that will audit all tasks, or in your case will explicitly disable auditing for all tasks.

So does this mean that an "auditctl -a task,never" _should_ disable syscall auding? I'm not 100% clear on this still.

Add a new spot in the assembly which will call a function which will check if audit_n_rules > 0 and if so will set TIF_SYSCALL_AUDIT and if not will clear TIF_SYSCALL_AUDIT? It might make things slightly worse on systems which explictly disable audit and the flag would always be clear on every task (like you did with the explicit rule) but I'm guessing might be a win on systems with no rules which are wasting time on the audit slow path.....

Quoted 7 lines · ----- "Eric Paris

Add a new spot in the assembly which will call a function which will check if audit_n_rules > 0 and if so will set TIF_SYSCALL_AUDIT and if not will clear TIF_SYSCALL_AUDIT? It might make things slightly worse on systems which explictly disable audit and the flag would always be clear on every task (like you did with the explicit rule) but I'm guessing might be a win on systems with no rules which are wasting time on the audit slow path.....

Is "audit_n_rules" a specific enough trigger? Right now, even if there are no rules configured at all, audit_log_start() while processing a syscall will mark that syscall for auditing, and all collected information about the syscall will be logged at syscall exit.

Would the suggested change break this behavior?

Quoted 4 lines · Michael Neuling

Quoted 3 lines · In message <[email protected]> you wrote:

Quoted 2 lines · Michael Neuling

We found that when auditing is disabled using "auditctl -D", that there's still a significant overhead when doing syscalls. This overhea

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines

is not present when a single never rule is inserted using "auditctl -a task,never".

Using Anton's null syscall microbenchmark from
http://ozlabs.org/~anton/junkcode/null_syscall.c we currently have on a powerpc machine:

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     739.03 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

This doesn't seem right, as we'd hope that auditing would have the same minimal impact when disabled via -D as when we have a single never rule

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

The patch below creates a fast path when initialising a task. If the rules list for tasks is empty (the disabled -D option), we mark auditin

Quoted 19 lines

Quoted 19 lines

Quoted 19 lines

as disabled for this task.

When this is applied, our null syscall benchmark improves in the disabled case to match the single never rule case.

  # auditctl -D
  No rules
  # ./null_syscall
	  null_syscall:     204.62 cycles     100.00%
  # auditctl -a task,never
  # ./null_syscall
	  null_syscall:     204.63 cycles     100.00%

Reported-by Anton Blanchard <[email protected]>

Signed-off-by Michael Neuling <[email protected]>

---
I'm not familiar with the auditing code/infrastructure so I may have misunderstood something here

Quoted 5 lines

Quoted 5 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct t

Quoted 5 lines

kernel/auditsc.c +0 -0

index 1b31c13..1cd6ec7 100644
@@ -666,6 +666,11 @@ static enum audit_state audit_filter_task(struct t

Quoted 10 lines

struct *tsk, char **key)

Quoted 9 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct t · line 674

Quoted 9 lines

kernel/auditsc.c +5 -0

 	enum audit_state   state;
 
 	rcu_read_lock();
+	/* Fast path.  If the list is empty, disable auditing */
+	if (list_empty(&audit_filter_list[AUDIT_FILTER_TASK])) {
+		rcu_read_unlock();
+		return AUDIT_DISABLED;
+	}
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK

Quoted 2 lines

{

Quoted 1 line

Quoted 1 line

kernel/auditsc.c +0 -0

 		if (audit_filter_rules(tsk, &e->rule, NULL, NULL, &stat

Quoted 23 lines

Quoted 7 lines

kernel/auditsc.c static enum audit_state audit_filter_task(struct t · line 676

Quoted 1 line

kernel/auditsc.c +0 -0

 			if (state == AUDIT_RECORD_CONTEXT)

I don't think this works at all. I don't see how syscall audit'ing can work. What if I have nothing in the AUDIT_FILTER_TASK list but I want to audit all 'open(2)' syscalls? This patch is going to leave the task in the DISABLED state and we won't ever be able to match on the syscall rules.

Sorry my bad. I'm not too familiar with the audit infrastructure.

On reflection, we might have a bug in audit_alloc though. Currently we have this:

int audit_alloc(struct task_struct *tsk)

  {
	  <snip>
	  state = audit_filter_task(tsk, &key);
	  if (likely(state == AUDIT_DISABLED))
		  return 0;

	  <snip>
	  set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
	  return 0;

Quoted 7 lines

}

This gets called on fork. If we have "task,never" rule, we hit this state == AUDIT_DISABLED path, return immediately and the tasks TIF_SYSCALL_AUDIT flags doesn't get set. On powerpc, we check TIF_SYSCALL_AUDIT in asm on syscall entry to fast path not calling the syscall audit code.

I'm guessing it actually bypasses audit if the flag is not set? So we might have a bug, but i'd be surprised since I think we tested audit on powerpc....

Quoted 3 lines

This seems wrong to me as a "never" _task_ audit rule shouldn't effect _syscall_ auditing? Is there some interaction between task and syscall auditing that I'm missing?

There are 3 states for a given task, I don't remember the names off the top of my head, so I'll guess with: on, off, build. 'Build' is the state most processes usually live in. In this state we collect audit information about the task during the whole syscall and then we might (likely) throw that information away at syscall exit.

Some types of audit rule, which alter this state, can be checked at either 'entry' or 'exit' (first rule wins) At syscall entry we only have enough information (questionable if we even have enough information at all but that's a different question) to filter based on the task. You can create rules that will audit all tasks, or in your case will explicitly disable auditing for all tasks.

Normally a process would be in the default 'build' state after syscall entry, we will collect information about the syscall, and then we will check syscall rules at exit. Once you explicitly say 'I do not want any audit messages for this task' you are in 'off' instead of 'build.'

Quoted 7 lines

Quoted 4 lines

I wonder if you could get much back, in terms of performance, by moving the

         context->dummy = !audit_n_rules;

line to the top and just returning if context->dummy == 1;

We get 668.09 cycles with this optimisation, so it comes down a bit, but no where near if the auditing is disabled altogether.

Clean that patch up and send it. Sounds like a win no matter what else we do.

Quoted 4 lines

Like I said above, powerpc has a fast path in asm on system call entry to check the thread_info flags for if syscall auditing is disabled. If it's disabled, we don't call the audit code, hence why it's very fast in this case.

Here's a new idea to think about with obvious tradeoffs. What do you think about doing a little bit of assembly rejiggering?

Add a new spot in the assembly which will call a function which will check if audit_n_rules > 0 and if so will set TIF_SYSCALL_AUDIT and if not will clear TIF_SYSCALL_AUDIT? It might make things slightly worse on systems which explictly disable audit and the flag would always be clear on every task (like you did with the explicit rule) but I'm guessing might be a win on systems with no rules which are wasting time on the audit slow path.....

BTW, do you think we can do this in audit_syscall_exit() too?

If I do, I get down to 387 cycles (739.03 vanilla, 668.09 with audit_syscall_entry() optimisation, 204 best case) so about another 50% perf improvement.

Patch was simply:

kernel/auditsc.c +1 -1

@@ -1681,7 +1683,7 @@ void audit_syscall_exit(int valid, long 
 
 	context = audit_get_context(tsk, valid, return_code);
 
-	if (likely(!context))
+	if (likely((!context) || (audit_n_rules == 0)))
 		return;
 
 	if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)

Hi Eric,

Here's another approach Mikey and I were discussing. We allocate the tsk->audit_context as before, but we avoid setting the TIF_SYSCALL_AUDIT until the first rule gets added.

We could look at clearing the flag when the rules go back to zero, but this simple patch covers the most common case I think.

I don't think this works at all. I don't see how syscall audit'ing can work. What if I have nothing in the AUDIT_FILTER_TASK list but I want to audit all 'open(2)' syscalls? This patch is going to leave the task in the DISABLED state and we won't ever be able to match on the syscall rules.

Hi Eric,

Quoted 5 lines

I don't think this works at all. I don't see how syscall audit'ing can work. What if I have nothing in the AUDIT_FILTER_TASK list but I want to audit all 'open(2)' syscalls? This patch is going to leave the task in the DISABLED state and we won't ever be able to match on the syscall rules.

That's a good point. What if we went through and created an audit context for each thread at the point where we add a rule to the audit subsystem?

That would make the common case where no one touches audit go fast. It's only once you add a rule that you get the syscall entry/exit overhead of audit.

Anton