On Sat 07-05-22 11:00:28, Amir Goldstein wrote:
> Dirent events (create/delete/move) are only reported on watched
> directory inodes, but in fanotify as well as in legacy inotify, it was
> always allowed to set them on non-dir inode, which does not result in
> any meaningful outcome.
>
> Until kernel v5.17, dirent events in fanotify also differed from events
> "on child" (e.g. FAN_OPEN) in the information provided in the event.
> For example, FAN_OPEN could be set in the mask of a non-dir or the mask
> of its parent and event would report the fid of the child regardless of
> the marked object.
> By contrast, FAN_DELETE is not reported if the child is marked and the
> child fid was not reported in the events.
>
> Since kernel v5.17, with fanotify group flag FAN_REPORT_TARGET_FID, the
> fid of the child is reported with dirent events, like events "on child",
> which may create confusion for users expecting the same behavior as
> events "on child" when setting events in the mask on a child.
>
> The desired semantics of setting dirent events in the mask of a child
> are not clear, so for now, deny this action for a group initialized
> with flag FAN_REPORT_TARGET_FID and for the new event FAN_RENAME.
> We may relax this restriction in the future if we decide on the
> semantics and implement them.
>
> Fixes: d61fd650e9d2 ("fanotify: introduce group flag FAN_REPORT_TARGET_FID")
> Fixes: 8cc3b1ccd930 ("fanotify: wire up FAN_RENAME event")
> Link: https://lore.kernel.org/linux-fsdevel/20220505133057.zm5t6vumc4xdcnsg@xxxxxxxxxx/
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> Jan,
>
> Having slept over it, I think we should apply this stronger fix.
> I could have done this as an extra patch, but since FAN_RENAME was
> merged together with FAN_REPORT_TARGET_FID I did not see the point,
> but you could apply this on top of the FAN_RENAME patch if you prefer.
OK, done and force-pushed everywhere.
Honza
> Changes since v1:
> - Deny all dirent events on non-dir not only FAN_RENAME
> - Return -ENOTDIR instead of -EINVAL
>
> fs/notify/fanotify/fanotify_user.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index edad67d674dc..cf587ba2ba92 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1695,6 +1695,19 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> else
> mnt = path.mnt;
>
> + /*
> + * FAN_RENAME is not allowed on non-dir (for now).
> + * We shouldn't have allowed setting any dirent events in mask of
> + * non-dir, but because we always allowed it, error only if group
> + * was initialized with the new flag FAN_REPORT_TARGET_FID.
> + */
> + ret = -ENOTDIR;
> + if (inode && !S_ISDIR(inode->i_mode) &&
> + ((mask & FAN_RENAME) ||
> + ((mask & FANOTIFY_DIRENT_EVENTS) &&
> + FAN_GROUP_FLAG(group, FAN_REPORT_TARGET_FID))))
> + goto path_put_and_out;
> +
> /* Mask out FAN_EVENT_ON_CHILD flag for sb/mount/non-dir marks */
> if (mnt || !S_ISDIR(inode->i_mode)) {
> mask &= ~FAN_EVENT_ON_CHILD;
> --
> 2.25.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
