On Wed, Mar 16, 2022 at 1:56 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Mon 14-03-22 11:28:23, Amir Goldstein wrote:
> > On Mon, Mar 14, 2022 at 10:47 AM Jan Kara <jack@xxxxxxx> wrote:
> > >
> > > On Sat 12-03-22 11:22:29, Srinivas wrote:
> > > > If a process calls fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_MOUNT,
> > > > FAN_OPEN_PERM, 0, "/mountpoint") no other directory exclusions can be
> > > > applied.
> > > >
> > > > However a path (file) exclusion can still be applied using
> > > >
> > > > fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_IGNORED_MASK |
> > > > FAN_MARK_IGNORED_SURV_MODIFY, FAN_OPEN_PERM | FAN_CLOSE_WRITE, AT_FDCWD,
> > > > "/tmp/fio/abc"); ===> path exclusion that works.
> > > >
> > > > I think the directory exclusion not working is a bug as otherwise AV
> > > > solutions cant exclude directories when using FAN_MARK_MOUNT.
> > > >
> > > > I believe the change should be simple since we are already supporting
> > > > path exclusions. So we should be able to add the same for the directory
> > > > inode.
> > > >
> > > > 215676 – fanotify Ignoring/Excluding a Directory not working with
> > > > FAN_MARK_MOUNT (kernel.org)
> > >
> > > Thanks for report! So I believe this should be fixed by commit 4f0b903ded
> > > ("fsnotify: fix merge with parent's ignored mask") which is currently
> > > sitting in my tree and will go to Linus during the merge (opening in a
> > > week).
> >
> > Actually, in a closer look, that fix alone is not enough.
> >
> > With the current upstream kernel this should work to exclude events
> > in a directory:
> >
> > fanotify_mark(fd, FAN_MARK_ADD, FAN_EVENT_ON_CHILD |
> > FAN_OPEN_PERM | FAN_CLOSE_WRITE,
> > AT_FDCWD, "/tmp/fio/");
> > fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_IGNORED_MASK |
> > FAN_MARK_IGNORED_SURV_MODIFY,
> > FAN_OPEN_PERM | FAN_CLOSE_WRITE,
> > AT_FDCWD, "/tmp/fio/");
>
> Thinking about this again, it could also be considered a bug (although
> convenient at times ;), that in current upstream kernel this combination of
> marks results in ignoring the OPEN_PERM & CLOSE_WRITE events on children of
> /tmp/fio, couldn't it? We probably should not consider the parent's ignore
> mask for events on children to maintain compatibility with older kernels?
> In theory it could bite someone unexpectedly...
>
> And to enable this convenient functionality we should rather introduce the
> new bit we've discussed. What do you think?
>
I kind of made this "convenient bug" public on several occasions.
Considering how useful this bug is, I think it is more likely that someone
will be bit by "fixing" the "bug" in a stable kernel that is it likely for
someone to be bit by the "bug".
Also, if taking away the "convenient bug" from stable kernels without
providing the new API is not very fair. and making sure that the "bug fix"
is not backported to any stable kernel is not sustainable.
IOW, I do not see good coming from changing behavior now.
We can document that behavior of ON_CHILD/ONDIR in ignored mask
is "undefined" and then make the behavior defined with a new
FAN_MARK_IGNORED_MASK_* flag.
Thanks,
Amir.
