On Fri 19-11-21 09:17:36, Amir Goldstein wrote:
> We do not want to report the dirfid+name of a directory whose
> inode/sb are not watched, because watcher may not have permissions
> to see the directory content.
>
> The FAN_MOVED_FROM/TO flags are used internally to indicate to
> fanotify_alloc_event() if we need to record only the old parent+name,
> only the new parent+name or both.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 57 ++++++++++++++++++++++++++++++-----
> 1 file changed, 50 insertions(+), 7 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 018b32a57702..c0a3fb1dd066 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -290,6 +290,7 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> __u32 marks_mask = 0, marks_ignored_mask = 0;
> __u32 test_mask, user_mask = FANOTIFY_OUTGOING_EVENTS |
> FANOTIFY_EVENT_FLAGS;
> + __u32 moved_mask = 0;
> 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;
> @@ -327,17 +328,44 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
> continue;
>
> /*
> - * If the event is on a child and this mark is on a parent not
> - * watching children, don't send it!
> + * In the special case of FAN_RENAME event, inode mark is the
> + * mark on the old dir and parent mark is the mark on the new
> + * dir. We do not want to report the dirfid+name of a directory
> + * whose inode/sb are not watched. The FAN_MOVE flags
> + * are used internally to indicate if we need to report only
> + * the old parent+name, only the new parent+name or both.
> */
> - if (type == FSNOTIFY_OBJ_TYPE_PARENT &&
> - !(mark->mask & FS_EVENT_ON_CHILD))
> + if (event_mask & FAN_RENAME) {
> + /* Old dir sb are watched - report old info */
> + if (type != FSNOTIFY_OBJ_TYPE_PARENT &&
> + (mark->mask & FAN_RENAME))
> + moved_mask |= FAN_MOVED_FROM;
> + /* New dir sb are watched - report new info */
> + if (type != FSNOTIFY_OBJ_TYPE_INODE &&
> + (mark->mask & FAN_RENAME))
> + moved_mask |= FAN_MOVED_TO;
> + } else if (type == FSNOTIFY_OBJ_TYPE_PARENT &&
> + !(mark->mask & FS_EVENT_ON_CHILD)) {
> + /*
> + * If the event is on a child and this mark is on
> + * a parent not watching children, don't send it!
> + */
> continue;
> + }
It feels a bit hacky to mix the "what info to report" into the mask
especially as otherwise perfectly valid flags. Can we perhaps have a
separate function to find this out (like fanotify_rename_info_report_mask()
or something like that) and use it in fanotify_alloc_event() or directly in
fanotify_handle_event() and pass the result to fanotify_alloc_event()?
That would seem a bit cleaner to me.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
