On Fri 24-06-22 14:32:24, Amir Goldstein wrote: > On Wed, Jun 22, 2022 at 6:52 PM Jan Kara <jack@xxxxxxx> wrote: > > > @@ -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 <amir73il@xxxxxxxxx> > 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 <jack@xxxxxxxx> SUSE Labs, CR