From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Paris Subject: Re: Kernel oops+crash on repeated auditd restarts Date: Tue, 24 Apr 2012 14:31:39 -0400 Message-ID: <1335292299.10352.3.camel@localhost> References: <1332983643.384.8.camel@localhost> <1333660021.2273.0.camel@localhost> <20120420231424.1836e56b@oc8526070481.ibm.com> <1335198376.8224.4.camel@localhost> <20120424021210.283cd4cd@oc8526070481.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120424021210.283cd4cd@oc8526070481.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Marcelo Cerri Cc: linux-audit@redhat.com List-Id: linux-audit@redhat.com On Tue, 2012-04-24 at 02:12 -0300, Marcelo Cerri wrote: > On Mon, 23 Apr 2012 12:26:16 -0400, Eric Paris 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.... -Eric