On Wed 04-04-18 23:42:18, Amir Goldstein wrote:
> When event on child inodes are sent to the parent inode mark and
> parent inode mark was not marked with FAN_EVENT_ON_CHILD, the event
> will not be delivered to the listener process. However, if the same
> process also has a mount mark, the event to the parent inode will be
> delivered regadless of the mount mark mask.
>
> This behavior is incorrect in the case where the mount mark mask does
> not contain the specific event type. For example, the process adds
> a mark on a directory with mask FAN_MODIFY (without FAN_EVENT_ON_CHILD)
> and a mount mark with mask FAN_CLOSE_NOWRITE (without FAN_ONDIR).
>
> A modify event on a file inside that directory (and inside that mount)
> should not create a FAN_MODIFY event, because neither of the marks
> requested to get that event on the file.
>
> Fixes: 1968f5eed54c ("fanotify: use both marks when possible")
> Cc: stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 34 +++++++++++++++-------------------
> 1 file changed, 15 insertions(+), 19 deletions(-)
>
> Jan,
>
> While working on the super block mark patches [1], I stumbbled on
> this bug. I figured I might as well send the fix out now.
>
> I have written an LTP test [2] to reproduce it.
Thanks for the patch. I've added it to my tree and will push it to Linus
(probably for rc2).
Honza
>
> Thanks,
> Amir.
>
> [1] https://github.com/amir73il/linux/commits/fanotify_sb
> [2] https://github.com/amir73il/ltp/commits/fanotify_sb
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 6702a6a0bbb5..e0e6a9d627df 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -92,7 +92,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
> u32 event_mask,
> const void *data, int data_type)
> {
> - __u32 marks_mask, marks_ignored_mask;
> + __u32 marks_mask = 0, marks_ignored_mask = 0;
> const struct path *path = data;
>
> pr_debug("%s: inode_mark=%p vfsmnt_mark=%p mask=%x data=%p"
> @@ -108,24 +108,20 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
> !d_can_lookup(path->dentry))
> return false;
>
> - if (inode_mark && vfsmnt_mark) {
> - marks_mask = (vfsmnt_mark->mask | inode_mark->mask);
> - marks_ignored_mask = (vfsmnt_mark->ignored_mask | inode_mark->ignored_mask);
> - } else if (inode_mark) {
> - /*
> - * if the event is for a child and this inode doesn't care about
> - * events on the child, don't send it!
> - */
> - if ((event_mask & FS_EVENT_ON_CHILD) &&
> - !(inode_mark->mask & FS_EVENT_ON_CHILD))
> - return false;
> - marks_mask = inode_mark->mask;
> - marks_ignored_mask = inode_mark->ignored_mask;
> - } else if (vfsmnt_mark) {
> - marks_mask = vfsmnt_mark->mask;
> - marks_ignored_mask = vfsmnt_mark->ignored_mask;
> - } else {
> - BUG();
> + /*
> + * if the event is for a child and this inode doesn't care about
> + * events on the child, don't send it!
> + */
> + if (inode_mark &&
> + (!(event_mask & FS_EVENT_ON_CHILD) ||
> + (inode_mark->mask & FS_EVENT_ON_CHILD))) {
> + marks_mask |= inode_mark->mask;
> + marks_ignored_mask |= inode_mark->ignored_mask;
> + }
> +
> + if (vfsmnt_mark) {
> + marks_mask |= vfsmnt_mark->mask;
> + marks_ignored_mask |= vfsmnt_mark->ignored_mask;
> }
>
> if (d_is_dir(path->dentry) &&
> --
> 2.7.4
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
