On Thu, May 31, 2018 at 12:43 PM, <henry.wilson@xxxxxxxxxxx> wrote:
> From: Henry Wilson <henry.wilson@xxxxxxxxxxx>
>
> inotify: Add flag IN_MASK_CREATE for inotify_add_watch()
>
> The flag IN_MASK_CREATE is introduced as a flag for inotiy_add_watch() which
> prevents inotify from modifying any existing watches when invoked. If the
> pathname specified in the call has a watched inode associated with it and
> IN_MASK_CREATE is specified, fail with an errno of EEXIST.
>
> Use of IN_MASK_CREATE with IN_MASK_ADD is reserved for future use and will return
> EINVAL.
>
> RATIONALE
>
> In the current implementation, there is no way to prevent inotify_add_watch()
> from modifying existing watch descriptors. Even if the caller keeps a record of
> all watch descriptors collected, this is only sufficient to detect that an
> existing watch descriptor may have been modified.
>
> The assumption that a particular path will map to the same inode over multiple
> calls to inotify_add_watch() cannot be made as files can be renamed or deleted.
> It is also not possible to assume that two distinct paths do no map to the same
> inode, due to hard-links or a dereferenced symbolic link. Further uses of
> inotify_add_watch() to revert the change may cause other watch descriptors to
> be modified or created, merely compunding the problem. There is currently no
> system call such as inotify_modify_watch() to explicity modify a watch
> descriptor, which would be able to revert unwanted changes. Thus the caller
> cannot guarantee to be able to revert any changes to existing watch decriptors.
>
> Additionally the caller cannot assume that the events that are associated with a
> watch descriptor are within the set requested, as any future calls to
> inotify_add_watch() may unintentionally modify a watch descriptor's mask. Thus
> it cannot currently be guaranteed that a watch descriptor will only generate
> events which have been requested. The program must filter events which come
> through its watch descriptor to within its expected range.
>
> Signed-off-by: Henry Wilson <henry.wilson@xxxxxxxxxxx>
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
See one suggestions below.
> ---
>
> EXTENSIONS
>
> A new system call inotify_modify_watch(fd, wd, mask) would be useful to modify
> an existing watch directly to avoid similar problems when modifying a watch
> descriptor's mask
>
> CHANGELOG
>
> v2: updated inotify_user_init() to the increased size of INOTIFY_ALL_BITS
> v3: renamed IN_ONLY_CREATE to IN_EXCL_ADD
> additional rational for the change introduced to commit
> v4: renamed IN_EXCL_ADD to IN_MASK_CREATE
> EINVAL returned when attempting to use IN_MASK_CREATE and IN_MASK_ADD
> ---
> fs/notify/inotify/inotify_user.c | 9 ++++++++-
> include/linux/inotify.h | 2 +-
> include/uapi/linux/inotify.h | 1 +
> 3 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> index ef32f3657958..134e9c5308c8 100644
> --- a/fs/notify/inotify/inotify_user.c
> +++ b/fs/notify/inotify/inotify_user.c
> @@ -506,6 +506,7 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
> __u32 old_mask, new_mask;
> __u32 mask;
> int add = (arg & IN_MASK_ADD);
> + int create = (arg & IN_MASK_CREATE);
> int ret;
>
Exclusive flag validation would look nicer here...
if (add && create)
return -EINVAL;
> mask = inotify_arg_to_mask(arg);
> @@ -513,6 +514,8 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
> fsn_mark = fsnotify_find_mark(&inode->i_fsnotify_marks, group);
> if (!fsn_mark)
> return -ENOENT;
> + else if (create)
> + return -EEXIST;
>
> i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark);
>
> @@ -714,6 +717,10 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
> if (unlikely(!f.file))
> return -EBADF;
>
> + /* IN_MASK_ADD and IN_MASK_CREATE don't make sense together */
> + if (unlikely((mask & IN_MASK_ADD) && (mask & IN_MASK_CREATE)))
> + return -EINVAL;
> +
..., but it may be safer practice
to keep it in the outer syscall interface, so not sure.
Thanks,
Amir.
