* [PATCH v2 0/2] New fanotify API for ignoring events
@ 2022-06-24 14:35 Amir Goldstein
2022-06-24 14:35 ` [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask Amir Goldstein
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Amir Goldstein @ 2022-06-24 14:35 UTC (permalink / raw)
To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, linux-api
Hi Jan,
As we discussed [1], here is the implementation of the new
FAN_MARK_IGNORE API, to try and sort the historic mess of
FAN_MARK_IGNORED_MASK.
To address the bug you pointed out in v1 review, I added
helpers fsnotify_effective_ignore_mask() and
fsnotify_mask_applicable().
fsnotify_effective_ignore_mask() is used also in send_to_group(),
but not fsnotify_mask_applicable().
We could also use fsnotify_mask_applicable() for mark->mask
in send_to_group(), but then we will need to set FS_ISDIR in masks
on inotify/dnotify/... marks, so I don't think it is worth it(?).
We may need to change the helper names though to clarify this
subtle detail.
See LTP tests [2] and man page draft [3].
Thanks,
Amir.
Changes since v1:
- Replace "sticky" semantics with EEXITS error (Jan)
- Simplify EINVAL checks (Jan)
- Add missing ignore mask accessors
- Add fsnotify_effective_ignore_mask() helper
[1] https://lore.kernel.org/linux-fsdevel/20220428123824.ssq72ovqg2nao5f4@quack3.lan/
[2] https://github.com/amir73il/ltp/commits/fan_mark_ignore
[3] https://github.com/amir73il/man-pages/commits/fan_mark_ignore
Amir Goldstein (2):
fanotify: prepare for setting event flags in ignore mask
fanotify: introduce FAN_MARK_IGNORE
fs/notify/fanotify/fanotify.c | 19 ++++----
fs/notify/fanotify/fanotify.h | 2 +
fs/notify/fanotify/fanotify_user.c | 61 +++++++++++++++++-------
fs/notify/fdinfo.c | 6 +--
fs/notify/fsnotify.c | 21 +++++----
include/linux/fanotify.h | 5 +-
include/linux/fsnotify_backend.h | 74 +++++++++++++++++++++++++++---
include/uapi/linux/fanotify.h | 2 +
8 files changed, 147 insertions(+), 43 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask 2022-06-24 14:35 [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein @ 2022-06-24 14:35 ` Amir Goldstein 2022-06-26 7:57 ` Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 2/2] fanotify: introduce FAN_MARK_IGNORE Amir Goldstein 2022-06-26 15:57 ` [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2 siblings, 1 reply; 8+ messages in thread From: Amir Goldstein @ 2022-06-24 14:35 UTC (permalink / raw) To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, linux-api 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> --- fs/notify/fanotify/fanotify.c | 19 ++++---- fs/notify/fanotify/fanotify_user.c | 21 +++++---- fs/notify/fdinfo.c | 6 +-- fs/notify/fsnotify.c | 21 +++++---- include/linux/fsnotify_backend.h | 74 +++++++++++++++++++++++++++--- 5 files changed, 106 insertions(+), 35 deletions(-) diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c index 4f897e109547..cd7d09a569ff 100644 --- a/fs/notify/fanotify/fanotify.c +++ b/fs/notify/fanotify/fanotify.c @@ -295,12 +295,13 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group, const void *data, int data_type, struct inode *dir) { - __u32 marks_mask = 0, marks_ignored_mask = 0; + __u32 marks_mask = 0, marks_ignore_mask = 0; __u32 test_mask, user_mask = FANOTIFY_OUTGOING_EVENTS | FANOTIFY_EVENT_FLAGS; const struct path *path = fsnotify_data_path(data, data_type); unsigned int fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS); struct fsnotify_mark *mark; + bool ondir = event_mask & FAN_ONDIR; int type; pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n", @@ -315,19 +316,21 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group, return 0; } else if (!(fid_mode & FAN_REPORT_FID)) { /* Do we have a directory inode to report? */ - if (!dir && !(event_mask & FS_ISDIR)) + if (!dir && !ondir) return 0; } fsnotify_foreach_iter_mark_type(iter_info, mark, type) { - /* Apply ignore mask regardless of mark's ISDIR flag */ - marks_ignored_mask |= mark->ignored_mask; + /* + * Apply ignore mask depending on event flags in ignore mask. + */ + marks_ignore_mask |= + fsnotify_effective_ignore_mask(mark, ondir, type); /* - * If the event is on dir and this mark doesn't care about - * events on dir, don't send it! + * Send the event depending on event flags in mark mask. */ - if (event_mask & FS_ISDIR && !(mark->mask & FS_ISDIR)) + if (!fsnotify_mask_applicable(mark->mask, ondir, type)) continue; marks_mask |= mark->mask; @@ -336,7 +339,7 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group, *match_mask |= 1U << type; } - test_mask = event_mask & marks_mask & ~marks_ignored_mask; + test_mask = event_mask & marks_mask & ~marks_ignore_mask; /* * For dirent modification events (create/delete/move) that do not carry diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index c2255b440df9..b718df84bd56 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -1012,7 +1012,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, if (!(flags & FAN_MARK_IGNORED_MASK)) { fsn_mark->mask &= ~mask; } else { - fsn_mark->ignored_mask &= ~mask; + fsn_mark->ignore_mask &= ~mask; } newmask = fsnotify_calc_mask(fsn_mark); /* @@ -1021,7 +1021,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, * changes to the mask. * Destroy mark when only umask bits remain. */ - *destroy = !((fsn_mark->mask | fsn_mark->ignored_mask) & ~umask); + *destroy = !((fsn_mark->mask | fsn_mark->ignore_mask) & ~umask); spin_unlock(&fsn_mark->lock); return oldmask & ~newmask; @@ -1090,7 +1090,7 @@ static bool fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark, /* * Setting FAN_MARK_IGNORED_SURV_MODIFY for the first time may lead to * the removal of the FS_MODIFY bit in calculated mask if it was set - * because of an ignored mask that is now going to survive FS_MODIFY. + * because of an ignore mask that is now going to survive FS_MODIFY. */ if ((fan_flags & FAN_MARK_IGNORED_MASK) && (fan_flags & FAN_MARK_IGNORED_SURV_MODIFY) && @@ -1123,7 +1123,7 @@ static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark, if (!(fan_flags & FAN_MARK_IGNORED_MASK)) fsn_mark->mask |= mask; else - fsn_mark->ignored_mask |= mask; + fsn_mark->ignore_mask |= mask; recalc = fsnotify_calc_mask(fsn_mark) & ~fsnotify_conn_mask(fsn_mark->connector); @@ -1261,7 +1261,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group, /* * If some other task has this inode open for write we should not add - * an ignored mark, unless that ignored mark is supposed to survive + * an ignore mask, unless that ignore mask is supposed to survive * modification changes anyway. */ if ((flags & FAN_MARK_IGNORED_MASK) && @@ -1540,7 +1540,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, __kernel_fsid_t __fsid, *fsid = NULL; u32 valid_mask = FANOTIFY_EVENTS | FANOTIFY_EVENT_FLAGS; unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS; - bool ignored = flags & FAN_MARK_IGNORED_MASK; + bool ignore = flags & FAN_MARK_IGNORED_MASK; unsigned int obj_type, fid_mode; u32 umask = 0; int ret; @@ -1589,8 +1589,11 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, if (mask & ~valid_mask) return -EINVAL; - /* Event flags (ONDIR, ON_CHILD) are meaningless in ignored mask */ - if (ignored) + /* + * Event flags (FAN_ONDIR, FAN_EVENT_ON_CHILD) have no effect with + * FAN_MARK_IGNORED_MASK. + */ + if (ignore) mask &= ~FANOTIFY_EVENT_FLAGS; f = fdget(fanotify_fd); @@ -1717,7 +1720,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, * events with parent/name info for non-directory. */ if ((fid_mode & FAN_REPORT_DIR_FID) && - (flags & FAN_MARK_ADD) && !ignored) + (flags & FAN_MARK_ADD) && !ignore) mask |= FAN_EVENT_ON_CHILD; } diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c index 59fb40abe33d..55081ae3a6ec 100644 --- a/fs/notify/fdinfo.c +++ b/fs/notify/fdinfo.c @@ -113,7 +113,7 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) return; seq_printf(m, "fanotify ino:%lx sdev:%x mflags:%x mask:%x ignored_mask:%x ", inode->i_ino, inode->i_sb->s_dev, - mflags, mark->mask, mark->ignored_mask); + mflags, mark->mask, mark->ignore_mask); show_mark_fhandle(m, inode); seq_putc(m, '\n'); iput(inode); @@ -121,12 +121,12 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) struct mount *mnt = fsnotify_conn_mount(mark->connector); seq_printf(m, "fanotify mnt_id:%x mflags:%x mask:%x ignored_mask:%x\n", - mnt->mnt_id, mflags, mark->mask, mark->ignored_mask); + mnt->mnt_id, mflags, mark->mask, mark->ignore_mask); } else if (mark->connector->type == FSNOTIFY_OBJ_TYPE_SB) { struct super_block *sb = fsnotify_conn_sb(mark->connector); seq_printf(m, "fanotify sdev:%x mflags:%x mask:%x ignored_mask:%x\n", - sb->s_dev, mflags, mark->mask, mark->ignored_mask); + sb->s_dev, mflags, mark->mask, mark->ignore_mask); } } diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index 0b3e74935cb4..8687562df2e3 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -324,7 +324,8 @@ static int send_to_group(__u32 mask, const void *data, int data_type, struct fsnotify_group *group = NULL; __u32 test_mask = (mask & ALL_FSNOTIFY_EVENTS); __u32 marks_mask = 0; - __u32 marks_ignored_mask = 0; + __u32 marks_ignore_mask = 0; + bool is_dir = mask & FS_ISDIR; struct fsnotify_mark *mark; int type; @@ -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; } } @@ -344,14 +345,15 @@ static int send_to_group(__u32 mask, const void *data, int data_type, fsnotify_foreach_iter_mark_type(iter_info, mark, type) { group = mark->group; marks_mask |= mark->mask; - marks_ignored_mask |= mark->ignored_mask; + marks_ignore_mask |= + fsnotify_effective_ignore_mask(mark, is_dir, type); } - pr_debug("%s: group=%p mask=%x marks_mask=%x marks_ignored_mask=%x data=%p data_type=%d dir=%p cookie=%d\n", - __func__, group, mask, marks_mask, marks_ignored_mask, + pr_debug("%s: group=%p mask=%x marks_mask=%x marks_ignore_mask=%x data=%p data_type=%d dir=%p cookie=%d\n", + __func__, group, mask, marks_mask, marks_ignore_mask, data, data_type, dir, cookie); - if (!(test_mask & marks_mask & ~marks_ignored_mask)) + if (!(test_mask & marks_mask & ~marks_ignore_mask)) return 0; if (group->ops->handle_event) { @@ -423,7 +425,8 @@ static bool fsnotify_iter_select_report_types( * But is *this mark* watching children? */ if (type == FSNOTIFY_ITER_TYPE_PARENT && - !(mark->mask & FS_EVENT_ON_CHILD)) + !(mark->mask & FS_EVENT_ON_CHILD) && + !(fsnotify_ignore_mask(mark) & FS_EVENT_ON_CHILD)) continue; fsnotify_iter_set_report_type(iter_info, type); @@ -532,8 +535,8 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir, /* - * If this is a modify event we may need to clear some ignored masks. - * In that case, the object with ignored masks will have the FS_MODIFY + * If this is a modify event we may need to clear some ignore masks. + * In that case, the object with ignore masks will have the FS_MODIFY * event in its mask. * Otherwise, return if none of the marks care about this type of event. */ diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h index 9560734759fa..d6d60a097070 100644 --- a/include/linux/fsnotify_backend.h +++ b/include/linux/fsnotify_backend.h @@ -518,8 +518,8 @@ struct fsnotify_mark { struct hlist_node obj_list; /* Head of list of marks for an object [mark ref] */ struct fsnotify_mark_connector *connector; - /* Events types to ignore [mark->lock, group->mark_mutex] */ - __u32 ignored_mask; + /* Events types and flags to ignore [mark->lock, group->mark_mutex] */ + __u32 ignore_mask; /* General fsnotify mark flags */ #define FSNOTIFY_MARK_FLAG_ALIVE 0x0001 #define FSNOTIFY_MARK_FLAG_ATTACHED 0x0002 @@ -529,6 +529,7 @@ struct fsnotify_mark { /* fanotify mark flags */ #define FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY 0x0100 #define FSNOTIFY_MARK_FLAG_NO_IREF 0x0200 +#define FSNOTIFY_MARK_FLAG_IGNORE_FLAGS 0x0400 unsigned int flags; /* flags [mark->lock] */ }; @@ -655,15 +656,76 @@ extern void fsnotify_remove_queued_event(struct fsnotify_group *group, /* functions used to manipulate the marks attached to inodes */ -/* Get mask for calculating object interest taking ignored mask into account */ +/* + * Canonical "ignore mask" including event flags. + * + * Note the subtle semantic difference from the legacy ->ignored_mask. + * ->ignored_mask traditionally only meant which events should be ignored, + * while ->ignore_mask also includes flags regarding the type of objects on + * which events should be ignored. + */ +static inline __u32 fsnotify_ignore_mask(struct fsnotify_mark *mark) +{ + __u32 ignore_mask = mark->ignore_mask; + + /* The event flags in ignore mask take effect */ + if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORE_FLAGS) + return ignore_mask; + + /* + * Legacy behavior: + * - Always ignore events on dir + * - Ignore events on child if parent is watching children + */ + ignore_mask |= FS_ISDIR; + ignore_mask &= ~FS_EVENT_ON_CHILD; + ignore_mask |= mark->mask & FS_EVENT_ON_CHILD; + return ignore_mask; +} + +/* + * Check if mask (or ignore mask) should be applied depending if victim is a + * directory and whether it is reported to a watching parent. + */ +static inline bool fsnotify_mask_applicable(__u32 mask, bool is_dir, + int iter_type) +{ + /* Should mask be applied to a directory? */ + if (is_dir && !(mask & FS_ISDIR)) + return false; + + /* Should mask be applied to a child? */ + if (iter_type == FSNOTIFY_ITER_TYPE_PARENT && + !(mask & FS_EVENT_ON_CHILD)) + return false; + + return true; +} + +/* + * Effective ignore mask taking into account if event victim is a + * directory and whether it is reported to a watching parent. + */ +static inline __u32 fsnotify_effective_ignore_mask(struct fsnotify_mark *mark, + bool is_dir, int iter_type) +{ + __u32 ignore_mask = fsnotify_ignore_mask(mark); + + if (!fsnotify_mask_applicable(ignore_mask, is_dir, iter_type)) + return 0; + + return mark->ignore_mask & ALL_FSNOTIFY_EVENTS; +} + +/* Get mask for calculating object interest taking ignore mask into account */ static inline __u32 fsnotify_calc_mask(struct fsnotify_mark *mark) { __u32 mask = mark->mask; - if (!mark->ignored_mask) + if (!(mark->ignore_mask & ALL_FSNOTIFY_EVENTS)) return mask; - /* Interest in FS_MODIFY may be needed for clearing ignored mask */ + /* Interest in FS_MODIFY may be needed for clearing ignore mask */ if (!(mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)) mask |= FS_MODIFY; @@ -671,7 +733,7 @@ static inline __u32 fsnotify_calc_mask(struct fsnotify_mark *mark) * If mark is interested in ignoring events on children, the object must * show interest in those events for fsnotify_parent() to notice it. */ - return mask | (mark->ignored_mask & ALL_FSNOTIFY_EVENTS); + return mask | mark->ignore_mask; } /* Get mask of events for a list of marks */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask 2022-06-24 14:35 ` [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask Amir Goldstein @ 2022-06-26 7:57 ` Amir Goldstein 2022-06-27 11:32 ` Jan Kara 0 siblings, 1 reply; 8+ messages in thread From: Amir Goldstein @ 2022-06-26 7:57 UTC (permalink / raw) To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Linux API On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@gmail.com> 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@gmail.com> > --- [...] > @@ -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. 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? 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. In any case, I am going to post v3 with my API proposal, but considering the buggy v1 and API issue in v2, I will need to improve the test coverage before FAN_MARK_IGNORE can be merged. Thanks, Amir. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask 2022-06-26 7:57 ` Amir Goldstein @ 2022-06-27 11:32 ` Jan Kara 2022-06-27 12:14 ` Amir Goldstein 0 siblings, 1 reply; 8+ messages in thread From: Jan Kara @ 2022-06-27 11:32 UTC (permalink / raw) To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel, Linux API On Sun 26-06-22 10:57:46, Amir Goldstein wrote: > On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@gmail.com> 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@gmail.com> > > --- > > [...] > > > @@ -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@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask 2022-06-27 11:32 ` Jan Kara @ 2022-06-27 12:14 ` Amir Goldstein 0 siblings, 0 replies; 8+ messages in thread From: Amir Goldstein @ 2022-06-27 12:14 UTC (permalink / raw) To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Linux API On Mon, Jun 27, 2022 at 2:32 PM Jan Kara <jack@suse.cz> wrote: > > On Sun 26-06-22 10:57:46, Amir Goldstein wrote: > > On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@gmail.com> 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@gmail.com> > > > --- > > > > [...] > > > > > @@ -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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] fanotify: introduce FAN_MARK_IGNORE 2022-06-24 14:35 [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask Amir Goldstein @ 2022-06-24 14:35 ` Amir Goldstein 2022-06-26 15:57 ` [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2 siblings, 0 replies; 8+ messages in thread From: Amir Goldstein @ 2022-06-24 14:35 UTC (permalink / raw) To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, linux-api This flag is a new way to configure ignore mask which allows adding and removing the event flags FAN_ONDIR and FAN_EVENT_ON_CHILD in ignore mask. The legacy FAN_MARK_IGNORED_MASK flag would always ignore events on directories and would ignore events on children depending on whether the FAN_EVENT_ON_CHILD flag was set in the (non ignored) mask. FAN_MARK_IGNORE can be used to ignore events on children without setting FAN_EVENT_ON_CHILD in the mark's mask and will not ignore events on directories unconditionally, only when FAN_ONDIR is set in ignore mask. The new behavior is non-downgradable. After calling fanotify_mark() with FAN_MARK_IGNORE once, calling fanotify_mark() with FAN_MARK_IGNORED_MASK on the same object will return EEXIST error. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/notify/fanotify/fanotify.h | 2 ++ fs/notify/fanotify/fanotify_user.c | 44 ++++++++++++++++++++++++------ include/linux/fanotify.h | 5 +++- include/uapi/linux/fanotify.h | 2 ++ 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h index 80e0ec95b113..c54d0b404373 100644 --- a/fs/notify/fanotify/fanotify.h +++ b/fs/notify/fanotify/fanotify.h @@ -499,6 +499,8 @@ static inline unsigned int fanotify_mark_user_flags(struct fsnotify_mark *mark) mflags |= FAN_MARK_IGNORED_SURV_MODIFY; if (mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF) mflags |= FAN_MARK_EVICTABLE; + if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORE_FLAGS) + mflags |= FAN_MARK_IGNORE; return mflags; } diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index b718df84bd56..0a657ded48e4 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -1009,7 +1009,7 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark, mask &= ~umask; spin_lock(&fsn_mark->lock); oldmask = fsnotify_calc_mask(fsn_mark); - if (!(flags & FAN_MARK_IGNORED_MASK)) { + if (!(flags & FANOTIFY_MARK_IGNORE_BITS)) { fsn_mark->mask &= ~mask; } else { fsn_mark->ignore_mask &= ~mask; @@ -1085,15 +1085,24 @@ static bool fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark, unsigned int fan_flags) { bool want_iref = !(fan_flags & FAN_MARK_EVICTABLE); + unsigned int ignore = fan_flags & FANOTIFY_MARK_IGNORE_BITS; bool recalc = false; + /* + * When using FAN_MARK_IGNORE for the first time, mark starts using + * independent event flags in ignore mask. After that, trying to + * update the ignore mask with the old FAN_MARK_IGNORED_MASK API + * will result in EEXIST error. + */ + if (ignore == FAN_MARK_IGNORE) + fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORE_FLAGS; + /* * Setting FAN_MARK_IGNORED_SURV_MODIFY for the first time may lead to * the removal of the FS_MODIFY bit in calculated mask if it was set * because of an ignore mask that is now going to survive FS_MODIFY. */ - if ((fan_flags & FAN_MARK_IGNORED_MASK) && - (fan_flags & FAN_MARK_IGNORED_SURV_MODIFY) && + if (ignore && (fan_flags & FAN_MARK_IGNORED_SURV_MODIFY) && !(fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)) { fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY; if (!(fsn_mark->mask & FS_MODIFY)) @@ -1120,7 +1129,7 @@ static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark, bool recalc; spin_lock(&fsn_mark->lock); - if (!(fan_flags & FAN_MARK_IGNORED_MASK)) + if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS)) fsn_mark->mask |= mask; else fsn_mark->ignore_mask |= mask; @@ -1216,11 +1225,21 @@ static int fanotify_add_mark(struct fsnotify_group *group, goto out; } + /* + * New ignore mask semantics cannot be downgraded to old semantics. + */ + if (fan_flags & FAN_MARK_IGNORED_MASK && + fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORE_FLAGS) { + ret = -EEXIST; + goto out; + } + /* * Error events are pre-allocated per group, only if strictly * needed (i.e. FAN_FS_ERROR was requested). */ - if (!(fan_flags & FAN_MARK_IGNORED_MASK) && (mask & FAN_FS_ERROR)) { + if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS) && + (mask & FAN_FS_ERROR)) { ret = fanotify_group_init_error_pool(group); if (ret) goto out; @@ -1264,7 +1283,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group, * an ignore mask, unless that ignore mask is supposed to survive * modification changes anyway. */ - if ((flags & FAN_MARK_IGNORED_MASK) && + if ((flags & FANOTIFY_MARK_IGNORE_BITS) && !(flags & FAN_MARK_IGNORED_SURV_MODIFY) && inode_is_open_for_write(inode)) return 0; @@ -1540,7 +1559,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, __kernel_fsid_t __fsid, *fsid = NULL; u32 valid_mask = FANOTIFY_EVENTS | FANOTIFY_EVENT_FLAGS; unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS; - bool ignore = flags & FAN_MARK_IGNORED_MASK; + unsigned int ignore = flags & FANOTIFY_MARK_IGNORE_BITS; unsigned int obj_type, fid_mode; u32 umask = 0; int ret; @@ -1589,12 +1608,19 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask, if (mask & ~valid_mask) return -EINVAL; + + /* We don't allow FAN_MARK_IGNORE & FAN_MARK_IGNORED_MASK together */ + if (ignore == (FAN_MARK_IGNORE | FAN_MARK_IGNORED_MASK)) + return -EINVAL; + /* * Event flags (FAN_ONDIR, FAN_EVENT_ON_CHILD) have no effect with * FAN_MARK_IGNORED_MASK. */ - if (ignore) + if (ignore == FAN_MARK_IGNORED_MASK) { mask &= ~FANOTIFY_EVENT_FLAGS; + umask = FANOTIFY_EVENT_FLAGS; + } f = fdget(fanotify_fd); if (unlikely(!f.file)) @@ -1803,7 +1829,7 @@ static int __init fanotify_user_setup(void) BUILD_BUG_ON(FANOTIFY_INIT_FLAGS & FANOTIFY_INTERNAL_GROUP_FLAGS); BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 12); - BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 10); + BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11); fanotify_mark_cache = KMEM_CACHE(fsnotify_mark, SLAB_PANIC|SLAB_ACCOUNT); diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h index edc28555814c..f32893942fd7 100644 --- a/include/linux/fanotify.h +++ b/include/linux/fanotify.h @@ -59,12 +59,15 @@ #define FANOTIFY_MARK_TYPE_BITS (FAN_MARK_INODE | FAN_MARK_MOUNT | \ FAN_MARK_FILESYSTEM) +#define FANOTIFY_MARK_IGNORE_BITS (FAN_MARK_IGNORED_MASK | \ + FAN_MARK_IGNORE) + #define FANOTIFY_MARK_FLAGS (FANOTIFY_MARK_TYPE_BITS | \ + FANOTIFY_MARK_IGNORE_BITS | \ FAN_MARK_ADD | \ FAN_MARK_REMOVE | \ FAN_MARK_DONT_FOLLOW | \ FAN_MARK_ONLYDIR | \ - FAN_MARK_IGNORED_MASK | \ FAN_MARK_IGNORED_SURV_MODIFY | \ FAN_MARK_EVICTABLE | \ FAN_MARK_FLUSH) diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h index f1f89132d60e..3efe417f2282 100644 --- a/include/uapi/linux/fanotify.h +++ b/include/uapi/linux/fanotify.h @@ -83,6 +83,8 @@ #define FAN_MARK_FLUSH 0x00000080 /* FAN_MARK_FILESYSTEM is 0x00000100 */ #define FAN_MARK_EVICTABLE 0x00000200 +/* This bit is mutually exclusive with FAN_MARK_IGNORED_MASK bit */ +#define FAN_MARK_IGNORE 0x00000400 /* These are NOT bitwise flags. Both bits can be used togther. */ #define FAN_MARK_INODE 0x00000000 -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] New fanotify API for ignoring events 2022-06-24 14:35 [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 2/2] fanotify: introduce FAN_MARK_IGNORE Amir Goldstein @ 2022-06-26 15:57 ` Amir Goldstein 2022-06-27 11:24 ` Jan Kara 2 siblings, 1 reply; 8+ messages in thread From: Amir Goldstein @ 2022-06-26 15:57 UTC (permalink / raw) To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Linux API On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@gmail.com> wrote: > > Hi Jan, > > As we discussed [1], here is the implementation of the new > FAN_MARK_IGNORE API, to try and sort the historic mess of > FAN_MARK_IGNORED_MASK. > Jan, When we started talking about adding FAN_MARK_IGNORE it was to address one specific flaw of FAN_MARK_IGNORED_MASK, but after staring at the API for some time, I realized there are other wrinkles with FAN_MARK_IGNORED_MASK that could be addressed by a fresh new API. I added more input validations following the EEXIST that you requested. The new errors can be seen in the ERRORS section of the man page [3]. The new restrictions will reduce the size of the test matrix, but I did not update the LTP tests [2] to check the new restrictions yet. I do not plan to post v3 patches before improving the LTP tests, but I wanted to send this heads up as an API proposal review. The kernel commit that adds FAN_MARK_IGNORE [1] summarize the new API restrictions as follows: The new behavior is non-downgradable. After calling fanotify_mark() with FAN_MARK_IGNORE once, calling fanotify_mark() with FAN_MARK_IGNORED_MASK on the same object will return EEXIST error. Setting the event flags with FAN_MARK_IGNORE on a non-dir inode mark has no meaning and will return ENOTDIR error. The meaning of FAN_MARK_IGNORED_SURV_MODIFY is preserved with the new FAN_MARK_IGNORE flag, but with a few semantic differences: 1. FAN_MARK_IGNORED_SURV_MODIFY is required for filesystem and mount marks and on an inode mark on a directory. Omitting this flag will return EINVAL or EISDIR error. 2. An ignore mask on a non-directory inode that survives modify could never be downgraded to an ignore mask that does not survive modify. With new FAN_MARK_IGNORE semantics we make that rule explicit - trying to update a surviving ignore mask without the flag FAN_MARK_IGNORED_SURV_MODIFY will return EEXIST error. The conveniene macro FAN_MARK_IGNORE_SURV is added for (FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY), because the common case should use short constant names. Comments and questions on the API changes are most welcome. Suggestion for more restrictions as well. Thanks, Amir. [1] https://github.com/amir73il/linux/commits/fan_mark_ignore [2] https://github.com/amir73il/ltp/commits/fan_mark_ignore [3] https://github.com/amir73il/man-pages/commits/fan_mark_ignore ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] New fanotify API for ignoring events 2022-06-26 15:57 ` [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein @ 2022-06-27 11:24 ` Jan Kara 0 siblings, 0 replies; 8+ messages in thread From: Jan Kara @ 2022-06-27 11:24 UTC (permalink / raw) To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel, Linux API Hi Amir! On Sun 26-06-22 18:57:01, Amir Goldstein wrote: > On Fri, Jun 24, 2022 at 5:35 PM Amir Goldstein <amir73il@gmail.com> wrote: > > As we discussed [1], here is the implementation of the new > > FAN_MARK_IGNORE API, to try and sort the historic mess of > > FAN_MARK_IGNORED_MASK. > > When we started talking about adding FAN_MARK_IGNORE > it was to address one specific flaw of FAN_MARK_IGNORED_MASK, > but after staring at the API for some time, I realized there are other > wrinkles with FAN_MARK_IGNORED_MASK that could be addressed > by a fresh new API. > > I added more input validations following the EEXIST that you requested. > The new errors can be seen in the ERRORS section of the man page [3]. > The new restrictions will reduce the size of the test matrix, but I did not > update the LTP tests [2] to check the new restrictions yet. > > I do not plan to post v3 patches before improving the LTP tests, > but I wanted to send this heads up as an API proposal review. > The kernel commit that adds FAN_MARK_IGNORE [1] summarize the > new API restrictions as follows: > > The new behavior is non-downgradable. After calling fanotify_mark() with > FAN_MARK_IGNORE once, calling fanotify_mark() with FAN_MARK_IGNORED_MASK > on the same object will return EEXIST error. > > Setting the event flags with FAN_MARK_IGNORE on a non-dir inode mark > has no meaning and will return ENOTDIR error. > > The meaning of FAN_MARK_IGNORED_SURV_MODIFY is preserved with the new > FAN_MARK_IGNORE flag, but with a few semantic differences: > > 1. FAN_MARK_IGNORED_SURV_MODIFY is required for filesystem and mount > marks and on an inode mark on a directory. Omitting this flag > will return EINVAL or EISDIR error. > > 2. An ignore mask on a non-directory inode that survives modify could > never be downgraded to an ignore mask that does not survive modify. > With new FAN_MARK_IGNORE semantics we make that rule explicit - > trying to update a surviving ignore mask without the flag > FAN_MARK_IGNORED_SURV_MODIFY will return EEXIST error. > > The conveniene macro FAN_MARK_IGNORE_SURV is added for > (FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY), because the > common case should use short constant names. This looks good to me. Thanks for working on this. The only additional thing that occurred to me is that we might want to restrict events usable with FAN_MARK_IGNORE similarly as we did restrict non-sensical events with FAN_REPORT_TARGET_FID but after putting more thought into it I'm not sure it is such a great idea because it is not so obvious which events may be valid to ignore for a particular object and how is it with backward compatibility when a new type of event becomes possible for an object. Honza > [1] https://github.com/amir73il/linux/commits/fan_mark_ignore > [2] https://github.com/amir73il/ltp/commits/fan_mark_ignore > [3] https://github.com/amir73il/man-pages/commits/fan_mark_ignore -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-06-27 12:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-24 14:35 [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 1/2] fanotify: prepare for setting event flags in ignore mask Amir Goldstein 2022-06-26 7:57 ` Amir Goldstein 2022-06-27 11:32 ` Jan Kara 2022-06-27 12:14 ` Amir Goldstein 2022-06-24 14:35 ` [PATCH v2 2/2] fanotify: introduce FAN_MARK_IGNORE Amir Goldstein 2022-06-26 15:57 ` [PATCH v2 0/2] New fanotify API for ignoring events Amir Goldstein 2022-06-27 11:24 ` Jan Kara
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.