On Fri, Nov 26, 2021 at 5:14 PM Jan Kara <jack@xxxxxxx> wrote:
>
> 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.
I used fsnotify_iter_info *match_info arg to fanotify_group_event_mask()
to indicate the marks of this group that matched the event and passed
it into fanotify_alloc_event().
Thanks,
Amir.
