Re: [PATCH] fanotify: fix logic of events on child

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

 



On Wed 04-04-18 23:42:18, Amir Goldstein wrote:
> When event on child inodes are sent to the parent inode mark and
> parent inode mark was not marked with FAN_EVENT_ON_CHILD, the event
> will not be delivered to the listener process. However, if the same
> process also has a mount mark, the event to the parent inode will be
> delivered regadless of the mount mark mask.
> 
> This behavior is incorrect in the case where the mount mark mask does
> not contain the specific event type. For example, the process adds
> a mark on a directory with mask FAN_MODIFY (without FAN_EVENT_ON_CHILD)
> and a mount mark with mask FAN_CLOSE_NOWRITE (without FAN_ONDIR).
> 
> A modify event on a file inside that directory (and inside that mount)
> should not create a FAN_MODIFY event, because neither of the marks
> requested to get that event on the file.
> 
> Fixes: 1968f5eed54c ("fanotify: use both marks when possible")
> Cc: stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>  fs/notify/fanotify/fanotify.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> Jan,
> 
> While working on the super block mark patches [1], I stumbbled on
> this bug. I figured I might as well send the fix out now.
> 
> I have written an LTP test [2] to reproduce it.

Thanks for the patch. I've added it to my tree and will push it to Linus
(probably for rc2).

								Honza

> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/linux/commits/fanotify_sb
> [2] https://github.com/amir73il/ltp/commits/fanotify_sb
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 6702a6a0bbb5..e0e6a9d627df 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -92,7 +92,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
>  				       u32 event_mask,
>  				       const void *data, int data_type)
>  {
> -	__u32 marks_mask, marks_ignored_mask;
> +	__u32 marks_mask = 0, marks_ignored_mask = 0;
>  	const struct path *path = data;
>  
>  	pr_debug("%s: inode_mark=%p vfsmnt_mark=%p mask=%x data=%p"
> @@ -108,24 +108,20 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
>  	    !d_can_lookup(path->dentry))
>  		return false;
>  
> -	if (inode_mark && vfsmnt_mark) {
> -		marks_mask = (vfsmnt_mark->mask | inode_mark->mask);
> -		marks_ignored_mask = (vfsmnt_mark->ignored_mask | inode_mark->ignored_mask);
> -	} else if (inode_mark) {
> -		/*
> -		 * if the event is for a child and this inode doesn't care about
> -		 * events on the child, don't send it!
> -		 */
> -		if ((event_mask & FS_EVENT_ON_CHILD) &&
> -		    !(inode_mark->mask & FS_EVENT_ON_CHILD))
> -			return false;
> -		marks_mask = inode_mark->mask;
> -		marks_ignored_mask = inode_mark->ignored_mask;
> -	} else if (vfsmnt_mark) {
> -		marks_mask = vfsmnt_mark->mask;
> -		marks_ignored_mask = vfsmnt_mark->ignored_mask;
> -	} else {
> -		BUG();
> +	/*
> +	 * if the event is for a child and this inode doesn't care about
> +	 * events on the child, don't send it!
> +	 */
> +	if (inode_mark &&
> +	    (!(event_mask & FS_EVENT_ON_CHILD) ||
> +	     (inode_mark->mask & FS_EVENT_ON_CHILD))) {
> +		marks_mask |= inode_mark->mask;
> +		marks_ignored_mask |= inode_mark->ignored_mask;
> +	}
> +
> +	if (vfsmnt_mark) {
> +		marks_mask |= vfsmnt_mark->mask;
> +		marks_ignored_mask |= vfsmnt_mark->ignored_mask;
>  	}
>  
>  	if (d_is_dir(path->dentry) &&
> -- 
> 2.7.4
> 
-- 
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]