On Mon, Jun 27, 2022 at 2:32 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Sun 26-06-22 10:57:46, Amir Goldstein wrote:
> > On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > >
> > > Setting flags FAN_ONDIR FAN_EVENT_ON_CHILD in ignore mask has no effect.
> > > The FAN_EVENT_ON_CHILD flag in mask implicitly applies to ignore mask and
> > > ignore mask is always implicitly applied to events on directories.
> > >
> > > Define a mark flag that replaces this legacy behavior with logic of
> > > applying the ignore mask according to event flags in ignore mask.
> > >
> > > Implement the new logic to prepare for supporting an ignore mask that
> > > ignores events on children and ignore mask that does not ignore events
> > > on directories.
> > >
> > > To emphasize the change in terminology, also rename ignored_mask mark
> > > member to ignore_mask and use accessors to get only the effective
> > > ignored events or the ignored events and flags.
> > >
> > > This change in terminology finally aligns with the "ignore mask"
> > > language in man pages and in most of the comments.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> >
> > [...]
> >
> > > @@ -336,7 +337,7 @@ static int send_to_group(__u32 mask, const void *data, int data_type,
> > > fsnotify_foreach_iter_mark_type(iter_info, mark, type) {
> > > if (!(mark->flags &
> > > FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY))
> > > - mark->ignored_mask = 0;
> > > + mark->ignore_mask = 0;
> > > }
> > > }
> >
> > Doh! I missed (again) the case of:
> > !FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY && !FS_EVENT_ON_CHILD
> >
> > I was starting to look at a fix, but then I stopped to think about the
> > justification
> > for FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY on a directory.
> >
> > The man page does say:
> > "... the ignore mask is cleared when a modify event occurs for the ignored file
> > or directory."
> > But ignore mask on a parent never really worked when this man page was
> > written and there is no such thing as a "modify event" on the directory itself.
> >
> > Furthermore, let's look at the motivation for IGNORED_SURV_MODIFY -
> > it is meant (I think) to suppress open/access permission events on a file
> > whose content was already scanned for malware until the content of that
> > file is modified - an important use case.
> >
> > But can that use case be extended to all files in a directory?
> > In theory, anti-malware software could scan a directory and call it "clean"
> > until any of the files therein is modified. However, an infected file can also
> > be moved into the "clean" directory, so unless we introduce a flag
> > IGNORED_DOES_NOT_SURV_MOVED_TO, supporting
> > !IGNORED_SURV_MODIFY on a directory seems useless.
> >
> > That leads me to suggest the thing I like most - deprecate.
> > Until someone comes up with a case to justify !IGNORED_SURV_MODIFY
> > on a directory, trying to set FAN_MARK_IGNORE on a directory without
> > IGNORED_SURV_MODIFY will return EISDIR.
> >
> > We could also say that IGNORED_SURV_MODIFY is implied on
> > a directory, but I think the EISDIR option is cleaner and easier to
> > document - especially for the case of "upgrading" a directory mark
> > from FAN_MARK_IGNORED_MASK to new FAN_MARK_IGNORE.
> >
> > We could limit that behavior to an ignore mask with EVENT_ON_CHILD
> > but that will just complicate things for no good reason.
>
> I think all of the above was reflected in your proposal in another email
> and I agree.
>
> > Semi-related, we recently did:
> > ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
> > We could have also disallowed FAN_ONDIR and FAN_EVENT_ON_CHILD
> > on non-dir inode. Too bad I didn't see it.
> > Do you think that we can/should "fix" FAN_REPORT_TARGET_FID to include
> > those restrictions?
>
> Yes, I think we could still amend the behavior. It isn't upstream for long
Yeh and not in any LTS either.
> and the combination is non-sensical in the first place... In the worst case
> we can revert without too much harm here.
>
Good, because I did add this restriction to FAN_MARK_IGNORE
best if at least those behaviors are matching for these 2 configs.
> > I would certainly like to disallow dirent events and the extra dir flags
> > for setting FAN_MARK_IGNORE on a non-dir inode.
> >
> > I am going to be on two weeks vacation v5.19-rc5..v5.19-rc7,
> > so unless we have clear answers about the API questions above
> > early this week, FAN_MARK_IGNORE will probably have to wait
> > another cycle.
>
> I'm on vacation next week as well. Let's see whether we'll be able to get
> things into shape for the merge window...
I'll start with the FIxes: patch, so at least we can get that one
to 5.19.
Thanks,
Amir.
