[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

    bt $TIF_SYSCALL_AUDIT,%edx
    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.049 / U:7.920 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site