On Sun 26-06-22 10:57:46, Amir Goldstein wrote:

On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein [off-list ref] wrote:

quoted

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@gmail.com>
---

[...]

quoted

@@ -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 [off-list ref]
SUSE Labs, CR