[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [patch] fix syscall speedup patch mips typo


There have been several concerns raised about this patch, so I did
some digging to find out what's really happening here.

On Mon, Mar 06, 2006 at 10:32:01AM -0500, Jason Baron wrote:
> Patch is below. The idea behind this patch is based on a suggestion
> from Steve Grubb to not call 'audit_syscall_entry' and
> 'audit_syscall_exit' if there are no audit rules loaded. This is
> problematic for the case where audit_log() is called in the middle
> of a system call (since we don't have the entry parameters).

First, there are several problems with the AUDIT_SYSCALL_PARTIAL
record.  It is true that audit_log() may be called in the middle of a
syscall, but it does not depend on the entry parameters or anything
else in the audit_context.

The check that results in an AUDIT_SYSCALL_PARTIAL record was added in
audit_log_exit(), which is quite different from an audit_log() call.
In addition to being called on syscall exit, audit_log_exit() is
called when a task is destroyed or in the error path of
copy_process().  The idea being that if we've marked that we want to
log a record and we don't make it to syscall exit, we can still log
the record at that point.

So, how do we determine that we want to log a record?  One way is
through filtering, if any rules match something in the audit_context.
The other way is through audit_get_stamp() which is called as part of
audit_log().  When audit_get_stamp() is called, it determines the
timestamp and serial number for the record based on the existence of
an audit_context.  If there is no audit_context, it generates a new
timestamp and serial number.  If the audit_context exists, it uses the
context's timestamp and serial number so all records generated in a
given syscall will have the same identifiers.  In the latter case, it
also marks that we want to generate an audit record.

This is interesting because if you have audit_enabled=1, you will
always get an AUDIT_SYSCALL record for any syscall during which a
record was logged outside of audit (e.g.  SELinux).  If you have
audit_enabled=0, you get only the externally generated records.

This patch avoids the record that would be generated on syscall exit,
but then emits it as AUDIT_SYSCALL_PARTIAL when the task exits.
Previously, these contents would be logged on every syscall exit, and
would only be logged on task exit in the case that it happened prior
to syscall exit.

As a user, you need to decide if you want the syscall records or not.
If you want them, then you need to pay the performance penalty for
collecting their information and logging them when they are triggered.
This mechanism of trying to eliminate some of their necessary
processing and then logging them at incorrect times is just bogus.
The audit_enabled flag was created to turn off the overhead of
creating syscall records.  That is the mechanism that should be used
to achieve the 30% performance gain.

When you set audit_enabled=0, what will your log records look like?

For one, you won't see the syscall records discussed above, and I
would argue that users only interested in records generated by SELinux
don't care about these.  The other difference is that calls to
audit_get_loginuid() will return -1 since there is no audit_context.
As audit_get_loginuid() is called from other places besides SELinux,
the real solution is to extract audit's preserving of the loginuid
from the syscall auditing functionality.  This would allow a user with
no audit rules to set audit_enabled=0 and see the same SELinux records
as they currently do with audit_enabled=1, while achieving the 30%
performance improvement.  

Allowing a user to set audit_enabled manually gives them the
flexibility to determine whether they want to see the extra syscall
records mentioned above.  If we determined that a user with no rules
doesn't want to see *any* syscall records, it wouldn't be difficult
for the kernel to automatically set audit_enabled=0 when there are no
rules, and turn it back on when a rule is added.

Additionally, David Woodhouse did some work to separate the filtering
for userspace messages (AUDIT_USER, etc.) from the syscall filtering.
This patch is currently in the audit git tree.  By simply removing the
check for audit_enabled in audit_receive_msg(), we can allow userspace
to send messages, including AUDIT_USER_AVC messages, when syscall
auditing is disabled.

The justification for this patch (mostly discussed offlist) was that
many users are only interested in using audit to log records from
SELinux but don't want to incur the performance penalty of syscall
auditing.  Fixing the loginuid and allowing userspace messages in the
absence of syscall auditing should cover their requirements and allow
them to simply turn syscall auditing off.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]