lkml.org 
[lkml]   [2004]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Light-weight Auditing Framework
    On Thu, Mar 11, 2004 at 09:25:46AM -0500, Rik Faith wrote:
    > Below is a patch against 2.6.4 that provides a low-overhead system-call
    > auditing framework for Linux that is usable by LSM components (e.g.,
    > SELinux). This is an update of the patch discussed in this thread:
    > http://marc.theaimsgroup.com/?t=107815888100001&r=1&w=2

    [ . . . ]

    Great application of RCU -- audit rules should not change
    often, but could be referenced quite frequently!

    A couple of comments:

    o I don't see any rcu_read_lock() or rcu_read_unlock() calls.
    These are needed on the read side in order to (1) let the
    people reading the code know the extent of the read-side
    critical section and (2) disable preemption in CONFIG_PREEMPT
    kernels. Without the former, someone will end up putting
    a blocking primitive in the wrong place. Without the latter,
    the kernel will do the dirty work all by itself. Either way,
    you get breakage.

    For example, I suspect that audit_filter_task() needs to
    read as follows:

    static enum audit_state audit_filter_task(struct task_struct *tsk)
    {
    struct audit_entry *e;
    enum audit_state state;

    rcu_read_lock();
    list_for_each_entry_rcu(e, &audit_tsklist, list) {
    if (audit_filter_rules(tsk, &e->rule, NULL, &state)) {
    rcu_read_unlock();
    return state;
    }
    }
    rcu_read_unlock();
    return AUDIT_BUILD_CONTEXT;
    }

    Alternatively, you could put the rcu_read_lock() and
    rcu_read_unlock() around the single call to audit_filter_task()
    from audit_alloc().

    All of the list_for_each_.*_rcu() macros need to be enclosed
    by rcu_read_lock() and rcu_read_unlock() calls.

    o Presumably something surrounding netlink_kernel_create()
    ensures that only one instance of audit_del_rule() will
    be executing at a given time. If not, some locking is
    needed.

    Once this locking is present, the list_for_each_entry_rcu()
    in audit_del_rule() should be changed to list_for_each_entry(),
    as it cannot race with deletion, since it -is- deletion.
    The list_del_rcu() is correct, and should remain.

    If you are using some sort of implicit locking, then please
    inject a clue...

    o The audit_add_rule() function also needs something to prevent
    races with other audit_add_rule() and audit_del_rule()
    instances. Again, this might be happening somehow in
    the netlink_kernel_create() mechanism, but I don't
    immediately see it. Then again, I do not claim to fully
    understand how the netlink_kernel_create() mechanism
    functions.

    Again, looks like a promising application of RCU!

    Thanx, Paul
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:01    [W:3.773 / U:0.528 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site