Re: [PATCH 2/4] fsnotify: split fsnotify_perm() into two hooks

[Jan Kara <jack@suse.cz>]

On Thu 07-12-23 14:38:23, Amir Goldstein wrote:
> We would like to make changes to the fsnotify access permission hook -
> add file range arguments and add the pre modify event.
> 
> In preparation for these changes, split the fsnotify_perm() hook into
> fsnotify_open_perm() and fsnotify_file_perm().
> 
> This is needed for fanotify "pre content" events.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/fsnotify.h | 34 +++++++++++++++++++---------------
>  security/security.c      |  4 ++--
>  2 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index bcb6609b54b3..926bb4461b9e 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -100,29 +100,33 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
>  	return fsnotify_parent(path->dentry, mask, path, FSNOTIFY_EVENT_PATH);
>  }
>  
> -/* Simple call site for access decisions */
> -static inline int fsnotify_perm(struct file *file, int mask)
> +/*
> + * fsnotify_file_perm - permission hook before file access
> + */
> +static inline int fsnotify_file_perm(struct file *file, int perm_mask)
>  {
> -	int ret;
> -	__u32 fsnotify_mask = 0;
> +	__u32 fsnotify_mask = FS_ACCESS_PERM;
>  
> -	if (!(mask & (MAY_READ | MAY_OPEN)))
> +	if (!(perm_mask & MAY_READ))
>  		return 0;
>  
> -	if (mask & MAY_OPEN) {
> -		fsnotify_mask = FS_OPEN_PERM;
> +	return fsnotify_file(file, fsnotify_mask);
> +}
>  
> -		if (file->f_flags & __FMODE_EXEC) {
> -			ret = fsnotify_file(file, FS_OPEN_EXEC_PERM);
> +/*
> + * fsnotify_open_perm - permission hook before file open
> + */
> +static inline int fsnotify_open_perm(struct file *file)
> +{
> +	int ret;
>  
> -			if (ret)
> -				return ret;
> -		}
> -	} else if (mask & MAY_READ) {
> -		fsnotify_mask = FS_ACCESS_PERM;
> +	if (file->f_flags & __FMODE_EXEC) {
> +		ret = fsnotify_file(file, FS_OPEN_EXEC_PERM);
> +		if (ret)
> +			return ret;
>  	}
>  
> -	return fsnotify_file(file, fsnotify_mask);
> +	return fsnotify_file(file, FS_OPEN_PERM);
>  }
>  
>  /*
> diff --git a/security/security.c b/security/security.c
> index dcb3e7014f9b..d7f3703c5905 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2586,7 +2586,7 @@ int security_file_permission(struct file *file, int mask)
>  	if (ret)
>  		return ret;
>  
> -	return fsnotify_perm(file, mask);
> +	return fsnotify_file_perm(file, mask);
>  }
>  
>  /**
> @@ -2837,7 +2837,7 @@ int security_file_open(struct file *file)
>  	if (ret)
>  		return ret;
>  
> -	return fsnotify_perm(file, MAY_OPEN);
> +	return fsnotify_open_perm(file);
>  }
>  
>  /**
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR