Re: audit.47 filesystem problems (reported by Rob Myers)

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);