Re: [PATCH 10/15] fs: move permission hook out of do_iter_write()

[Christian Brauner <brauner@xxxxxxxxxx>]

On Tue, Nov 14, 2023 at 05:33:16PM +0200, Amir Goldstein wrote:
> In many of the vfs helpers, the rw_verity_area() checks are called before
> taking sb_start_write(), making them "start-write-safe".
> do_iter_write() is an exception to this rule.
> 
> do_iter_write() has two callers - vfs_iter_write() and vfs_writev().
> Move rw_verify_area() and other checks from do_iter_write() out to
> its callers to make them "start-write-safe".
> 
> Move also the fsnotify_modify() hook to align with similar pattern
> used in vfs_write() and other vfs helpers.
> 
> This is needed for fanotify "pre content" events.
> 
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>  fs/read_write.c | 76 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 46 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 8cdc6e6a9639..d4891346d42e 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -848,28 +848,10 @@ EXPORT_SYMBOL(vfs_iter_read);
>  static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
>  			     loff_t *pos, rwf_t flags)
>  {
> -	size_t tot_len;
> -	ssize_t ret = 0;
> -
> -	if (!(file->f_mode & FMODE_WRITE))
> -		return -EBADF;
> -	if (!(file->f_mode & FMODE_CAN_WRITE))
> -		return -EINVAL;
> -
> -	tot_len = iov_iter_count(iter);
> -	if (!tot_len)
> -		return 0;
> -	ret = rw_verify_area(WRITE, file, pos, tot_len);
> -	if (ret < 0)
> -		return ret;
> -
>  	if (file->f_op->write_iter)
> -		ret = do_iter_readv_writev(file, iter, pos, WRITE, flags);
> +		return do_iter_readv_writev(file, iter, pos, WRITE, flags);
>  	else
> -		ret = do_loop_readv_writev(file, iter, pos, WRITE, flags);
> -	if (ret > 0)
> -		fsnotify_modify(file);
> -	return ret;
> +		return do_loop_readv_writev(file, iter, pos, WRITE, flags);

Nit, this will end up being:

static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
                             loff_t *pos, rwf_t flags)
{
        if (file->f_op->write_iter)
                return do_iter_readv_writev(file, iter, pos, WRITE, flags);
        else
                return do_loop_readv_writev(file, iter, pos, WRITE, flags);
}

which is probably best written as:

static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
                             loff_t *pos, rwf_t flags)
{
        if (file->f_op->write_iter)
                return do_iter_readv_writev(file, iter, pos, WRITE, flags);

        return do_loop_readv_writev(file, iter, pos, WRITE, flags);
}