On Wed 22-11-23 14:27:02, Amir Goldstein wrote:
> vfs_splice_read() has a permission hook inside rw_verify_area() and
> it is called from do_splice_direct() -> splice_direct_to_actor().
>
> The callers of do_splice_direct() (e.g. vfs_copy_file_range()) already
> call rw_verify_area() for the entire range, but the other caller of
> splice_direct_to_actor() (nfsd) does not.
>
> Add the rw_verify_area() checks in nfsd_splice_read() and use a
> variant of vfs_splice_read() without rw_verify_area() check in
> splice_direct_to_actor() to avoid the redundant rw_verify_area() checks.
>
> This is needed for fanotify "pre content" events.
>
> Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/nfsd/vfs.c | 5 ++++-
> fs/splice.c | 58 +++++++++++++++++++++++++++++++--------------------
> 2 files changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index fbbea7498f02..5d704461e3b4 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1046,7 +1046,10 @@ __be32 nfsd_splice_read(struct svc_rqst *rqstp, struct svc_fh *fhp,
> ssize_t host_err;
>
> trace_nfsd_read_splice(rqstp, fhp, offset, *count);
> - host_err = splice_direct_to_actor(file, &sd, nfsd_direct_splice_actor);
> + host_err = rw_verify_area(READ, file, &offset, *count);
> + if (!host_err)
> + host_err = splice_direct_to_actor(file, &sd,
> + nfsd_direct_splice_actor);
> return nfsd_finish_read(rqstp, fhp, file, offset, count, eof, host_err);
> }
>
> diff --git a/fs/splice.c b/fs/splice.c
> index 6e917db6f49a..6fc2c27e9520 100644
> --- a/fs/splice.c
> +++ b/fs/splice.c
> @@ -944,27 +944,15 @@ static void do_splice_eof(struct splice_desc *sd)
> sd->splice_eof(sd);
> }
>
> -/**
> - * vfs_splice_read - Read data from a file and splice it into a pipe
> - * @in: File to splice from
> - * @ppos: Input file offset
> - * @pipe: Pipe to splice to
> - * @len: Number of bytes to splice
> - * @flags: Splice modifier flags (SPLICE_F_*)
> - *
> - * Splice the requested amount of data from the input file to the pipe. This
> - * is synchronous as the caller must hold the pipe lock across the entire
> - * operation.
> - *
> - * If successful, it returns the amount of data spliced, 0 if it hit the EOF or
> - * a hole and a negative error code otherwise.
> +/*
> + * Callers already called rw_verify_area() on the entire range.
> + * No need to call it for sub ranges.
> */
> -long vfs_splice_read(struct file *in, loff_t *ppos,
> - struct pipe_inode_info *pipe, size_t len,
> - unsigned int flags)
> +static long do_splice_read(struct file *in, loff_t *ppos,
> + struct pipe_inode_info *pipe, size_t len,
> + unsigned int flags)
> {
> unsigned int p_space;
> - int ret;
>
> if (unlikely(!(in->f_mode & FMODE_READ)))
> return -EBADF;
> @@ -975,10 +963,6 @@ long vfs_splice_read(struct file *in, loff_t *ppos,
> p_space = pipe->max_usage - pipe_occupancy(pipe->head, pipe->tail);
> len = min_t(size_t, len, p_space << PAGE_SHIFT);
>
> - ret = rw_verify_area(READ, in, ppos, len);
> - if (unlikely(ret < 0))
> - return ret;
> -
> if (unlikely(len > MAX_RW_COUNT))
> len = MAX_RW_COUNT;
>
> @@ -992,6 +976,34 @@ long vfs_splice_read(struct file *in, loff_t *ppos,
> return copy_splice_read(in, ppos, pipe, len, flags);
> return in->f_op->splice_read(in, ppos, pipe, len, flags);
> }
> +
> +/**
> + * vfs_splice_read - Read data from a file and splice it into a pipe
> + * @in: File to splice from
> + * @ppos: Input file offset
> + * @pipe: Pipe to splice to
> + * @len: Number of bytes to splice
> + * @flags: Splice modifier flags (SPLICE_F_*)
> + *
> + * Splice the requested amount of data from the input file to the pipe. This
> + * is synchronous as the caller must hold the pipe lock across the entire
> + * operation.
> + *
> + * If successful, it returns the amount of data spliced, 0 if it hit the EOF or
> + * a hole and a negative error code otherwise.
> + */
> +long vfs_splice_read(struct file *in, loff_t *ppos,
> + struct pipe_inode_info *pipe, size_t len,
> + unsigned int flags)
> +{
> + int ret;
> +
> + ret = rw_verify_area(READ, in, ppos, len);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + return do_splice_read(in, ppos, pipe, len, flags);
> +}
> EXPORT_SYMBOL_GPL(vfs_splice_read);
>
> /**
> @@ -1066,7 +1078,7 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
> size_t read_len;
> loff_t pos = sd->pos, prev_pos = pos;
>
> - ret = vfs_splice_read(in, &pos, pipe, len, flags);
> + ret = do_splice_read(in, &pos, pipe, len, flags);
> if (unlikely(ret <= 0))
> goto read_failure;
>
> --
> 2.34.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
