Re: Kernel oops+crash on repeated auditd restarts

[Marcelo Cerri <mhcerri@linux.vnet.ibm.com>]

On Mon, 23 Apr 2012 12:26:16 -0400
Eric Paris <eparis@redhat.com> wrote:

> On Fri, 2012-04-20 at 23:14 -0300, Marcelo Cerri wrote:
> 
> > The patch below increments the reference count of a mark when it is
> > added to the destroy list. It seems to solve the issue and it
> > doesn't seem to cause any memory leak. Please, can you make some
> > tests in your environments and let me know if there is any problem
> > with this patch.
> 
> That is almost certainly the wrong thing to do.  This test program
> should show a memory leak with your patch.  If it doesn't show a
> memory leak then something is screwed up in inotify as well.

Sorry, I should have tested the other features that also make use of
fsnotify. You're right, my patch adds a memory leak for inotify (and
probably for dnotify and fanotify too).

> ...
> 
> The lifetime of an object is supposed to be from fsnotify_init_mark()
> until it's matching reference is dropped in fsnotify_mark_destroy().
> It sounds to me like we are calling put somewhere in the audit code
> when we didn't previously call a get....
> 

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:
-- 
1.7.9.4