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

On Wed, 2005-03-23 at 14:22 -0600, Timothy R. Chavez wrote: 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(a)tycho.nsa.gov&gt; National Security Agency