On Fri 24-06-22 14:32:24, Amir Goldstein wrote:

On Wed, Jun 22, 2022 at 6:52 PM Jan Kara [off-list ref] wrote:

quoted

quoted

@@ -336,7 +341,7 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
              *match_mask |= 1U << type;
      }

-     test_mask = event_mask & marks_mask & ~marks_ignored_mask;
+     test_mask = event_mask & marks_mask & ~marks_ignore_mask;

Especially because here if say FAN_EVENT_ON_CHILD becomes a part of
marks_ignore_mask it can result in clearing this flag in the returned
'mask' which is likely not what we want if there are some events left
unignored in the 'mask'?

You are right.
This can end up clearing FAN_ONDIR and then we won't report it.
However, take a look at this:

commit 0badfa029e5fd6d5462adb767937319335637c83
Author: Amir Goldstein [off-list ref]
Date:   Thu Jul 16 11:42:09 2020 +0300

    fanotify: generalize the handling of extra event flags

    In fanotify_group_event_mask() there is logic in place to make sure we
    are not going to handle an event with no type and just FAN_ONDIR flag.
    Generalize this logic to any FANOTIFY_EVENT_FLAGS.

    There is only one more flag in this group at the moment -
    FAN_EVENT_ON_CHILD. We never report it to user, but we do pass it in to
    fanotify_alloc_event() when group is reporting fid as indication that
    event happened on child. We will have use for this indication later on.

What the hell did I mean by "We will have use for this indication later on"?

Heh, I was wondering about exactly that sentence when I was writing my
review comments and looking where the code came from :). I didn't find a
good explanation...

fanotify_alloc_event() does not look at the FAN_EVENT_ON_CHILD flag.
I think I had the idea that events reported in a group with FAN_REPORT_NAME
on an inode mark should not report its parent fid+name to be compatible with
inotify behavior and I think you shot this idea down, but it is only a guess.

Yeah, maybe.

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR