[RFC][PATCH] (#6 U1) the latest incarnation

[Stephen Smalley]

On Wed, 2005-03-23 at 14:22 -0600, Timothy R. Chavez wrote:
> This is the latest patch.

Other comments:

- As it stands with your patch, alloc_inode() will leak memory if
audit_inode_alloc() succeeds but security_inode_alloc() fails because
nothing frees the audit data on the error handling path for that case.
Also, I have to wonder whether it is truly necessary to make i_audit
conditionally compiled; note that i_security is always present and
initialized to NULL even if !CONFIG_SECURITY.  If you can make it
unconditional, then you can also initialize it to NULL prior to the
audit_inode_alloc() call so that you have a known initial state, and
then just call audit_inode_free() on the error handling path (which will
either map to free'ing NULL, which is safe, or free'ing an actual audit
data, avoiding the leak).  Or you could just make setting i_audit to
NULL the first step in audit_inode_alloc() prior to attempting the
allocation to ensure a known initial state.

- You've switched from storing the audit data in the current audit
context for later processing by audit_log_exit() to immediately
generation of partial audit data via audit_log* calls from
audit_notify_watch().  That goes beyond what I suggested; all I
suggested was setting the auditable flag in the current audit context
(while still saving the file information in the current audit context)
so that audit_syscall_exit will call audit_log_exit() and generate an
audit record upon syscall exit whenever an audit_notify_watch() is
triggered on an auditable inode during the syscall processing.  Your new
approach is more like what SELinux presently does, but seems counter to
what others have been suggesting, i.e. batch everything up for
processing on syscall exit.  Both approaches ensure that an audit record
is emitted whenever an auditable inode is encountered, but the present
approach yields two separate audit records (one immediate from your hook
and one upon syscall exit) vs. a single unified record.  What do we
want?  What do others think?

-- 
Stephen Smalley <sds at tycho.nsa.gov>
National Security Agency