Re: [PATCH resend] audit: fix mark refcounting

[Linus Torvalds]

Looks reasonable, but why doesn't all callers have that "put_mark()" thing?

And if/when all callers *do* have that put_mark() thing, maybe we
should make destroy_mark() just do it?

In particular, a quick grep shows that there are destroy_mark users still in:

 - fs/notify/fanotify/fanotify_user.c

 - fs/notify/dnotify/dnotify.c (2 of them)

 - fs/notify/inotify/inotify_fsnotify.c


that don't do "put_mark()" after the destroy. Why is it ok there?

I don't know the code, it's probably fine, but I'd like to know why
the audit code needs it but not the other sites (but my grep didn't
look at context)

And I'd like Al to say something. Al?

                      Linus

On Wed, Dec 14, 2011 at 6:35 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> I think this should go into 3.2 (and stable).
>
> Please apply.
>
> Thanks,
> Miklos
> ----
>
> From: Miklos Szeredi <mszeredi@xxxxxxx>
> Subject: audit: fix mark refcounting
>
> Removing the parent of a watched file results in "kernel BUG at
> fs/notify/mark.c:139".
>
> To reproduce
>
>  add "-w /tmp/audit/dir/watched_file" to audit.rules
>  rm -rf /tmp/audit/dir
>
> This is caused by fsnotify_destroy_mark() being called without an
> extra reference taken by the caller.
>
> Reported by Francesco Cosoleto here:
>
>  https://bugzilla.novell.com/show_bug.cgi?id=689860
>
> Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxx>
> CC: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> CC: Eric Paris <eparis@xxxxxxxxxx>
> CC: stable@xxxxxxxxxxxxxxx
> ---
>  kernel/audit_watch.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> Index: linux.git/kernel/audit_watch.c
> ===================================================================
> --- linux.git.orig/kernel/audit_watch.c 2011-12-06 16:39:48.000000000 +0100
> +++ linux.git/kernel/audit_watch.c      2011-12-13 14:05:37.000000000 +0100
> @@ -349,7 +349,9 @@ static void audit_remove_parent_watches(
>        }
>        mutex_unlock(&audit_filter_mutex);
>
> +       audit_get_parent(parent);
>        fsnotify_destroy_mark(&parent->mark);
> +       audit_put_parent(parent);
>  }
>
>  /* Get path information necessary for adding watches. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/