Re: Kernel oops+crash on repeated auditd restarts

[Eric Paris <eparis@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 <eparis@redhat.com> 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