linux-kernel - Re: [PATCH resend] audit: fix mark refcounting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Dec 2011 18:15:11 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Miklos Szeredi <miklos@...redi.hu>
Cc:	Eric Paris <eparis@...hat.com>, Al Viro <viro@...iv.linux.org.uk>,
	akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH resend] audit: fix mark refcounting

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@...redi.hu> wrote:
> I think this should go into 3.2 (and stable).
>
> Please apply.
>
> Thanks,
> Miklos
> ----
>
> From: Miklos Szeredi <mszeredi@...e.cz>
> 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@...e.cz>
> CC: Al Viro <viro@...iv.linux.org.uk>
> CC: Eric Paris <eparis@...hat.com>
> CC: stable@...r.kernel.org
> ---
>  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/