audit.47 filesystem problems (reported by Rob Myers)

[Timothy R. Chavez]

On Tuesday 24 May 2005 12:43, Timothy R. Chavez wrote:

<snip>

> Problem 2:
> 
> An OOPS when referencing a NULL pointer when watching /var/audit/audit.log in 
> an SMP environment which has not yet been reproduced.
> 
> Diagnosis:
> 
> Because this has not be reproducible and we're in an SMP environment its 
> probably safe to say there is a race.  Because the OOPs occurred from within 
> permission() and we're dealing with file system watches it is safe to assume 
> that the race occurred somewhere in or under audit_notify_watch().
> 
> Solution:
> 

Ok, just looking at audit_notify_watch() in kernel:auditsc.c,

int audit_notify_watch(struct inode *inode, int mask)
{
        int ret = 0;
        struct audit_context *context = current->audit_context;
        struct audit_aux_data_watched *ax;
        struct audit_wentry *wentry = NULL;
                                                                                                                                                                     
        if (likely(!audit_enabled))
                goto audit_notify_watch_exit;
                                                                                                                                                                     
        if (!inode)
                goto audit_notify_watch_exit;
                                                                                                                                                                     
Possibly in an SMP environment there could be a race here
where we're trying to get a reference to inode->i_audit->wentry,
we pass the check in audit_wentry_get() to check if it's still OK,
then another CPU changes it, such that it becomes invalid in
some way (ie: NULL).  With the read_lock() we force anything
that's interested in this i_audit->wentry to block until we got
the reference successfully.  Does this make sense?

        read_lock(&inode->i_audit->lock);
        wentry = audit_wentry_get(inode->i_audit->wentry);
        if (!wentry)
                goto audit_notify_watch_exit;
        read_unlock(&inode->i_audit->lock);


-tim