linux-kernel - Re: [PATCH 4.9 30/30] fanotify: fix handling of events on child sub-directory

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 4 Feb 2019 12:48:00 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>, Jan Kara <jack@...e.cz>
Subject: Re: [PATCH 4.9 30/30] fanotify: fix handling of events on child sub-directory

On Mon, Feb 4, 2019 at 12:44 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> 4.9-stable review patch.  If anyone has any objections, please let me know.
>
> ------------------
>
> From: Amir Goldstein <amir73il@...il.com>
>
> commit b469e7e47c8a075cc08bcd1e85d4365134bdcdd5 upstream.
>
> When an event is reported on a sub-directory and the parent inode has
> a mark mask with FS_EVENT_ON_CHILD|FS_ISDIR, the event will be sent to
> fsnotify() even if the event type is not in the parent mark mask
> (e.g. FS_OPEN).
>
> Further more, if that event happened on a mount or a filesystem with
> a mount/sb mark that does have that event type in their mask, the "on
> child" event will be reported on the mount/sb mark.  That is not
> desired, because user will get a duplicate event for the same action.
>
> Note that the event reported on the victim inode is never merged with
> the event reported on the parent inode, because of the check in
> should_merge(): old_fsn->inode == new_fsn->inode.
>
> Fix this by looking for a match of an actual event type (i.e. not just
> FS_ISDIR) in parent's inode mark mask and by not reporting an "on child"
> event to group if event type is only found on mount/sb marks.
>
> [backport hint: The bug seems to have always been in fanotify, but this
>                 patch will only apply cleanly to v4.19.y]
>

Same comment about this backport hint being misleading in the
context of the backport patch.

> Cc: <stable@...r.kernel.org> # v4.19
> Signed-off-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: Jan Kara <jack@...e.cz>
> [amir: backport to v4.9]
> Signed-off-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> ---
>  fs/notify/fsnotify.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -101,9 +101,9 @@ int __fsnotify_parent(struct path *path,
>         parent = dget_parent(dentry);
>         p_inode = parent->d_inode;
>
> -       if (unlikely(!fsnotify_inode_watches_children(p_inode)))
> +       if (unlikely(!fsnotify_inode_watches_children(p_inode))) {
>                 __fsnotify_update_child_dentry_flags(p_inode);
> -       else if (p_inode->i_fsnotify_mask & mask) {
> +       } else if (p_inode->i_fsnotify_mask & mask & ~FS_EVENT_ON_CHILD) {
>                 struct name_snapshot name;
>
>                 /* we are notifying a parent so come up with the new mask which
> @@ -207,6 +207,10 @@ int fsnotify(struct inode *to_tell, __u3
>         else
>                 mnt = NULL;
>
> +       /* An event "on child" is not intended for a mount mark */
> +       if (mask & FS_EVENT_ON_CHILD)
> +               mnt = NULL;
> +
>         /*
>          * Optimization: srcu_read_lock() has a memory barrier which can
>          * be expensive.  It protects walking the *_fsnotify_marks lists.
>
>