On Tue, Apr 24, 2012 at 11:31 AM, Eric Paris <[email protected]> wrote: > On Tue, 2012-04-24 at 02:12 -0300, Marcelo Cerri wrote: >> On Mon, 23 Apr 2012 12:26:16 -0400, Eric Paris <[email protected]> wrote: > >> Considering that the issue is specific to audit and it seems to occur >> only with watches on directories, I investigated the audit_tree.c file >> and found a probable cause. The untag_chunk() holds a reference to a >> mark at the begging of the function and releases it at the end of it (on >> the label out). However when it jumps to the "out" label, it calls >> fsnotify_put_mark once more. >> >> Peter and Valentin, can you test this new patch to check if it >> solves the oops problem? >> >> Eric, do you agree with this solution? >> >> Regards, >> Marcelo >> >> --- >> kernel/audit_tree.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c >> index 5bf0790..b5bd9f9 100644 >> --- a/kernel/audit_tree.c >> +++ b/kernel/audit_tree.c >> @@ -250,7 +250,6 @@ static void untag_chunk(struct node *p) >> spin_unlock(&hash_lock); >> spin_unlock(&entry->lock); >> fsnotify_destroy_mark(entry); >> - fsnotify_put_mark(entry); >> goto out; >> } >> >> @@ -293,7 +292,6 @@ static void untag_chunk(struct node *p) >> spin_unlock(&hash_lock); >> spin_unlock(&entry->lock); >> fsnotify_destroy_mark(entry); >> - fsnotify_put_mark(entry); >> goto out; >> >> Fallback: > > This looks right to me. The old audit logic before the switch to > fsnotify was: > - inotify_evict_watch(&chunk->watch); > - mutex_unlock(&chunk->watch.inode->inotify_mutex); > - put_inotify_watch(&chunk->watch); > > Which I changed to: > + spin_unlock(&entry->lock); > + fsnotify_destroy_mark_by_entry(entry); > + fsnotify_put_mark(entry); > > The difference being that inotify_evict_watch() took a reference on > chunk->watch, however fsnotify_destroy_mark_by_entry() does not. So the > fsnotify_put_mark() was incorrect. > > I'd love to hear testing results, and I'm going to try to figure out if > I screwed that up other places....
I'm testing this now. It looks good WRT the crash. I need to spend some more time testing be sure memory isn't leaking anywhere. > -Eric > -- Peter Moody Google 1.650.253.7306 Security Engineer pgp:0xC3410038 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
