On Wed 22-11-23 14:27:00, Amir Goldstein wrote:
> The main callers of do_splice_direct() also call rw_verify_area() for
> the entire range that is being copied, e.g. by vfs_copy_file_range()
> or do_sendfile() before calling do_splice_direct().
>
> The only caller that does not have those checks for entire range is
> ovl_copy_up_file(). In preparation for removing the checks inside
> do_splice_direct(), add rw_verify_area() call in ovl_copy_up_file().
>
> For extra safety, perform minimal sanity checks from rw_verify_area()
> for non negative offsets also in the copy up do_splice_direct() loop
> without calling the file permission hooks.
>
> This is needed for fanotify "pre content" events.
>
> 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/overlayfs/copy_up.c | 26 +++++++++++++++++++++++++-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 4382881b0709..106f8643af3b 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -230,6 +230,19 @@ static int ovl_copy_fileattr(struct inode *inode, const struct path *old,
> return ovl_real_fileattr_set(new, &newfa);
> }
>
> +static int ovl_verify_area(loff_t pos, loff_t pos2, loff_t len, loff_t totlen)
> +{
> + loff_t tmp;
> +
> + if (WARN_ON_ONCE(pos != pos2))
> + return -EIO;
> + if (WARN_ON_ONCE(pos < 0 || len < 0 || totlen < 0))
> + return -EIO;
> + if (WARN_ON_ONCE(check_add_overflow(pos, len, &tmp)))
> + return -EIO;
> + return 0;
> +}
> +
> static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry,
> struct file *new_file, loff_t len)
> {
> @@ -244,13 +257,20 @@ static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry,
> int error = 0;
>
> ovl_path_lowerdata(dentry, &datapath);
> - if (WARN_ON(datapath.dentry == NULL))
> + if (WARN_ON_ONCE(datapath.dentry == NULL) ||
> + WARN_ON_ONCE(len < 0))
> return -EIO;
>
> old_file = ovl_path_open(&datapath, O_LARGEFILE | O_RDONLY);
> if (IS_ERR(old_file))
> return PTR_ERR(old_file);
>
> + error = rw_verify_area(READ, old_file, &old_pos, len);
> + if (!error)
> + error = rw_verify_area(WRITE, new_file, &new_pos, len);
> + if (error)
> + goto out_fput;
> +
> /* Try to use clone_file_range to clone up within the same fs */
> ovl_start_write(dentry);
> cloned = do_clone_file_range(old_file, 0, new_file, 0, len, 0);
> @@ -309,6 +329,10 @@ static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry,
> }
> }
>
> + error = ovl_verify_area(old_pos, new_pos, this_len, len);
> + if (error)
> + break;
> +
> ovl_start_write(dentry);
> bytes = do_splice_direct(old_file, &old_pos,
> new_file, &new_pos,
> --
> 2.34.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
