On Tue, Mar 19, 2024 at 1:16 AM Sasha Levin <sashal@xxxxxxxxxx> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
> fsnotify: consistent behavior for parent not watching children
>
Sasha,
I am surprised that Chuck was not CCed on this series that he backported.
Chuck,
This is indeed a proper fix worth backporting.
Please follow up in adjusting the LTP test fanotify09 to update this
comment with the appropriate 5.15.y version:
* Test cases #6-#7 are regression tests for commit:
* (from v5.19, unlikely to be backported thus not in .tags):
*
* e730558adffb fanotify: consistent behavior for parent not
watching children
As well as this hackish test condition:
if (tc->ignore && tst_kvercmp(5, 19, 0) < 0) {
tst_res(TCONF, "ignored mask on parent dir has undefined "
"behavior on kernel < 5.19");
return;
}
Thanks,
Amir.
> to the 5.15-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> fsnotify-consistent-behavior-for-parent-not-watching.patch
> and it can be found in the queue-5.15 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
>
> commit dad4072413ac37a8bc79e8e4d0a9a3b2210a3ccb
> Author: Amir Goldstein <amir73il@xxxxxxxxx>
> Date: Wed May 11 22:02:13 2022 +0300
>
> fsnotify: consistent behavior for parent not watching children
>
> [ Upstream commit e730558adffb88a52e562db089e969ee9510184a ]
>
> The logic for handling events on child in groups that have a mark on
> the parent inode, but without FS_EVENT_ON_CHILD flag in the mask is
> duplicated in several places and inconsistent.
>
> Move the logic into the preparation of mark type iterator, so that the
> parent mark type will be excluded from all mark type iterations in that
> case.
>
> This results in several subtle changes of behavior, hopefully all
> desired changes of behavior, for example:
>
> - Group A has a mount mark with FS_MODIFY in mask
> - Group A has a mark with ignore mask that does not survive FS_MODIFY
> and does not watch children on directory D.
> - Group B has a mark with FS_MODIFY in mask that does watch children
> on directory D.
> - FS_MODIFY event on file D/foo should not clear the ignore mask of
> group A, but before this change it does
>
> And if group A ignore mask was set to survive FS_MODIFY:
> - FS_MODIFY event on file D/foo should be reported to group A on account
> of the mount mark, but before this change it is wrongly ignored
>
> Fixes: 2f02fd3fa13e ("fanotify: fix ignore mask logic for events on child and on dir")
> Reported-by: Jan Kara <jack@xxxxxxxx>
> Link: https://lore.kernel.org/linux-fsdevel/20220314113337.j7slrb5srxukztje@xxxxxxxxxx/
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> Link: https://lore.kernel.org/r/20220511190213.831646-3-amir73il@xxxxxxxxx
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 263d303d8f8f1..4f897e1095470 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -320,7 +320,7 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> }
>
> fsnotify_foreach_iter_mark_type(iter_info, mark, type) {
> - /* Apply ignore mask regardless of ISDIR and ON_CHILD flags */
> + /* Apply ignore mask regardless of mark's ISDIR flag */
> marks_ignored_mask |= mark->ignored_mask;
>
> /*
> @@ -330,14 +330,6 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> if (event_mask & FS_ISDIR && !(mark->mask & FS_ISDIR))
> continue;
>
> - /*
> - * If the event is on a child and this mark is on a parent not
> - * watching children, don't send it!
> - */
> - if (type == FSNOTIFY_ITER_TYPE_PARENT &&
> - !(mark->mask & FS_EVENT_ON_CHILD))
> - continue;
> -
> marks_mask |= mark->mask;
>
> /* Record the mark types of this group that matched the event */
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 35740a64ee453..0b3e74935cb4f 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -290,22 +290,15 @@ static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
> }
>
> if (parent_mark) {
> - /*
> - * parent_mark indicates that the parent inode is watching
> - * children and interested in this event, which is an event
> - * possible on child. But is *this mark* watching children and
> - * interested in this event?
> - */
> - if (parent_mark->mask & FS_EVENT_ON_CHILD) {
> - ret = fsnotify_handle_inode_event(group, parent_mark, mask,
> - data, data_type, dir, name, 0);
> - if (ret)
> - return ret;
> - }
> - if (!inode_mark)
> - return 0;
> + ret = fsnotify_handle_inode_event(group, parent_mark, mask,
> + data, data_type, dir, name, 0);
> + if (ret)
> + return ret;
> }
>
> + if (!inode_mark)
> + return 0;
> +
> if (mask & FS_EVENT_ON_CHILD) {
> /*
> * Some events can be sent on both parent dir and child marks
> @@ -422,8 +415,19 @@ static bool fsnotify_iter_select_report_types(
> iter_info->report_mask = 0;
> fsnotify_foreach_iter_type(type) {
> mark = iter_info->marks[type];
> - if (mark && mark->group == iter_info->current_group)
> + if (mark && mark->group == iter_info->current_group) {
> + /*
> + * FSNOTIFY_ITER_TYPE_PARENT indicates that this inode
> + * is watching children and interested in this event,
> + * which is an event possible on child.
> + * But is *this mark* watching children?
> + */
> + if (type == FSNOTIFY_ITER_TYPE_PARENT &&
> + !(mark->mask & FS_EVENT_ON_CHILD))
> + continue;
> +
> fsnotify_iter_set_report_type(iter_info, type);
> + }
> }
>
> return true;
