Re: [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux