Re: [PATCH] fanotify: fix handling of events on child sub-directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue 30-10-18 20:29:53, Amir Goldstein wrote:
> When an event is reported on a sub-directory and the parent inode has
> a mark mask with FS_EVENT_ON_CHILD|FS_ISDIR, the event will be sent to
> fsnotify() even if the event type is not in the parent mark mask
> (e.g. FS_OPEN).
> 
> Further more, if that event happened on a mount or a filesystem with
> a mount/sb mark that does have that event type in their mask, the "on
> child" event will be reported on the mount/sb mark.  That is not
> desired, because user will get a duplicate event for the same action.
> 
> Note that the event reported on the victim inode is never merged with
> the event reported on the parent inode, because of the check in
> should_merge(): old_fsn->inode == new_fsn->inode.
> 
> Fix this by looking for a match of an actual event type (i.e. not just
> FS_ISDIR) in parent's inode mark mask and by not reporting an "on child"
> event to group if event type is only found on mount/sb marks.
> 
> [backport hint: The bug seems to have always been in fanotify, but this
>                 patch will only apply cleanly to v4.19.y]
> 
> Cc: <stable@xxxxxxxxxxxxxxx> # v4.19
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>

Thanks for the patch. I've added it to my tree.

								Honza

> ---
> 
> Jan,
> 
> The bugs with merging directory children mark and mount mark are
> unfolding slowly.
> 
> I have extended fanotify09, which checks another bug on this type to also
> cover this case [1].
> Tested that bug existed as far back as v4.10, but I guess it was always
> there.
> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/ltp/commits/fanotify_ignore
> 
>  fs/notify/fanotify/fanotify.c | 10 +++++-----
>  fs/notify/fsnotify.c          |  7 +++++--
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 5769cf3ff035..e08a6647267b 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -115,12 +115,12 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
>  			continue;
>  		mark = iter_info->marks[type];
>  		/*
> -		 * if the event is for a child and this inode doesn't care about
> -		 * events on the child, don't send it!
> +		 * If the event is for a child and this mark doesn't care about
> +		 * events on a child, don't send it!
>  		 */
> -		if (type == FSNOTIFY_OBJ_TYPE_INODE &&
> -		    (event_mask & FS_EVENT_ON_CHILD) &&
> -		    !(mark->mask & FS_EVENT_ON_CHILD))
> +		if (event_mask & FS_EVENT_ON_CHILD &&
> +		    (type != FSNOTIFY_OBJ_TYPE_INODE ||
> +		     !(mark->mask & FS_EVENT_ON_CHILD)))
>  			continue;
>  
>  		marks_mask |= mark->mask;
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 2172ba516c61..d2c34900ae05 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -167,9 +167,9 @@ int __fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask
>  	parent = dget_parent(dentry);
>  	p_inode = parent->d_inode;
>  
> -	if (unlikely(!fsnotify_inode_watches_children(p_inode)))
> +	if (unlikely(!fsnotify_inode_watches_children(p_inode))) {
>  		__fsnotify_update_child_dentry_flags(p_inode);
> -	else if (p_inode->i_fsnotify_mask & mask) {
> +	} else if (p_inode->i_fsnotify_mask & mask & ALL_FSNOTIFY_EVENTS) {
>  		struct name_snapshot name;
>  
>  		/* we are notifying a parent so come up with the new mask which
> @@ -339,6 +339,9 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is,
>  		sb = mnt->mnt.mnt_sb;
>  		mnt_or_sb_mask = mnt->mnt_fsnotify_mask | sb->s_fsnotify_mask;
>  	}
> +	/* An event "on child" is not intended for a mount/sb mark */
> +	if (mask & FS_EVENT_ON_CHILD)
> +		mnt_or_sb_mask = 0;
>  
>  	/*
>  	 * Optimization: srcu_read_lock() has a memory barrier which can
> -- 
> 2.17.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux