Unverified Commit 853b8d75 authored by Amir Goldstein's avatar Amir Goldstein Committed by Christian Brauner
Browse files

remap_range: merge do_clone_file_range() into vfs_clone_file_range() 



commit dfad3705 ("remap_range: move permission hooks out of
do_clone_file_range()") moved the permission hooks from
do_clone_file_range() out to its caller vfs_clone_file_range(),
but left all the fast sanity checks in do_clone_file_range().

This makes the expensive security hooks be called in situations
that they would not have been called before (e.g. fs does not support
clone).

The only reason for the do_clone_file_range() helper was that overlayfs
did not use to be able to call vfs_clone_file_range() from copy up
context with sb_writers lock held.  However, since commit c63e56a4
("ovl: do not open/llseek lower file with upper sb_writers held"),
overlayfs just uses an open coded version of vfs_clone_file_range().

Merge_clone_file_range() into vfs_clone_file_range(), restoring the
original order of checks as it was before the regressing commit and adapt
the overlayfs code to call vfs_clone_file_range() before the permission
hooks that were added by commit ca7ab482 ("ovl: add permission hooks
outside of do_splice_direct()").

Note that in the merge of do_clone_file_range(), the file_start_write()
context was reduced to cover ->remap_file_range() without holding it
over the permission hooks, which was the reason for doing the regressing
commit in the first place.

Reported-and-tested-by: default avatarkernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202401312229.eddeb9a6-oliver.sang@intel.com


Fixes: dfad3705 ("remap_range: move permission hooks out of do_clone_file_range()")
Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/r/20240202102258.1582671-1-amir73il@gmail.com


Reviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 6613476e

fs/overlayfs/copy_up.c

+6 −8
Original line number Diff line number Diff line
@@ -265,20 +265,18 @@ static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry, 
	if (IS_ERR(old_file))

		return PTR_ERR(old_file);



	/* Try to use clone_file_range to clone up within the same fs */

	cloned = vfs_clone_file_range(old_file, 0, new_file, 0, len, 0);

	if (cloned == len)

		goto out_fput;



	/* Couldn't clone, so now we try to copy the data */

	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);

	ovl_end_write(dentry);

	if (cloned == len)

		goto out_fput;

	/* Couldn't clone, so now we try to copy the data */



	/* Check if lower fs supports seek operation */

	if (old_file->f_mode & FMODE_LSEEK)

		skip_hole = true;

fs/remap_range.c

+9 −22
Original line number Diff line number Diff line
@@ -373,7 +373,7 @@ int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in, 
}

EXPORT_SYMBOL(generic_remap_file_range_prep);



loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,

loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,

			    struct file *file_out, loff_t pos_out,

			    loff_t len, unsigned int remap_flags)

{
@@ -391,23 +391,6 @@ loff_t do_clone_file_range(struct file *file_in, loff_t pos_in, 
	if (!file_in->f_op->remap_file_range)

		return -EOPNOTSUPP;



	ret = file_in->f_op->remap_file_range(file_in, pos_in,

			file_out, pos_out, len, remap_flags);

	if (ret < 0)

		return ret;



	fsnotify_access(file_in);

	fsnotify_modify(file_out);

	return ret;

}

EXPORT_SYMBOL(do_clone_file_range);



loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,

			    struct file *file_out, loff_t pos_out,

			    loff_t len, unsigned int remap_flags)

{

	loff_t ret;



	ret = remap_verify_area(file_in, pos_in, len, false);

	if (ret)

		return ret;
@@ -417,10 +400,14 @@ loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in, 
		return ret;



	file_start_write(file_out);

	ret = do_clone_file_range(file_in, pos_in, file_out, pos_out, len,

				  remap_flags);

	ret = file_in->f_op->remap_file_range(file_in, pos_in,

			file_out, pos_out, len, remap_flags);

	file_end_write(file_out);

	if (ret < 0)

		return ret;



	fsnotify_access(file_in);

	fsnotify_modify(file_out);

	return ret;

}

EXPORT_SYMBOL(vfs_clone_file_range);

include/linux/fs.h

+0 −3
Original line number Diff line number Diff line
@@ -2101,9 +2101,6 @@ int __generic_remap_file_range_prep(struct file *file_in, loff_t pos_in, 
int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in,

				  struct file *file_out, loff_t pos_out,

				  loff_t *count, unsigned int remap_flags);

extern loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,

				  struct file *file_out, loff_t pos_out,

				  loff_t len, unsigned int remap_flags);

extern loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,

				   struct file *file_out, loff_t pos_out,

				   loff_t len, unsigned int remap_flags);