[lkml]   [2010]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] audit: speedup for syscalls when auditing is disabled
> > 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.
> 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....

So on powerpc we have this in entry_64.S


andi. r11,r10,_TIF_SYSCALL_T_OR_A
bne- syscall_dotrace

and there is similar code in x86 in entry_64.S

jc sysret_audit

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?

> > 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.

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

> 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.'


> > > 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.

Ok, sending in separate email.

> > 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.....

This sounds good to me except for one thing...

If we set TIF_SYSCALL_AUDIT when audit_n_rules > 0, it'll change the
functionality from what we have now in the "auditctl -a task,never"
case. Currently in this case, TIF_SYSCALL_AUDIT will not be set and
syscalls won't be audited. I think this is a bug (as discussed above),
but I wanted to point it out anyway.

Anyway, I'll take a stab at this in a bit.


 \ /
  Last update: 2010-08-24 07:59    [W:0.047 / U:3.016 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site