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
and the combination is non-sensical in the first place... In the worst case
we can revert without too much harm here.
> 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...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
