On Thu 31-05-18 13:38:47, Amir Goldstein wrote: > 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. Yeah, I think the outer interface is better for clarity. I'll take the patch as is. Thanks for your review. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
