[RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Amir Goldstein]

Hi all,

Daan De Meyer requested a way to do copy_file_range() from overlayfs
to/from another fs, where if that fs is the base fs of overlayfs layer
the copy could be an efficient clone.

TBH, he also requested support also for cross-fs clone_file_range(),
but I am not happy about providing that.

copy_file_range() already supports cross-sb efficient copy (e.g. with
NFS/SMB server-side copy).

The current limitation of staying on the same fstype is mainly an
internal technical limitation (choosing the right f_op), which is
something that this RFC is trying to address.

The overlayfs implementation itself is pretty trivial.

Thanks,
Amir.


Amir Goldstein (5):
  fs: clarify cross-sb copy_file_range() code
  fs: add support for copy file range from another fs
  fs: add support for copy file range to another fs
  ovl: add support for copy file range from another fs
  ovl: add support for copy file range to another fs

 fs/overlayfs/file.c      |  35 ++++++++++++
 fs/overlayfs/ovl_entry.h |   7 ++-
 fs/read_write.c          | 119 ++++++++++++++++++++++++++++-----------
 include/linux/fs.h       |   2 +
 4 files changed, 130 insertions(+), 33 deletions(-)

-- 
2.54.0

[RFC][PATCH 1/5] fs: clarify cross-sb copy_file_range() code

[Amir Goldstein]

The cross-sb copy file range code is a bit complicated and spreads across
two different functions.

Use a helper copy_file_fs_cmp() an enum and switch statement to clarify
the mutual exclusive types of copy.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/read_write.c | 70 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index e8c14e2760b20..d1565a56d089d 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1472,6 +1472,35 @@ COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd,
 }
 #endif
 
+enum {
+	FS_COPY_SPLICE = 0,
+	FS_COPY_SAME_SB = 1,
+	FS_COPY_SAME_FS = 2,
+	FS_COPY_CROSS_FS = 3,
+};
+
+/*
+ * Check if src and dst file are on the same filesystem.
+ * nfs and cifs define several different file_system_type structures
+ * and several different sets of file_operations, but they all end up
+ * using the same ->copy_file_range() function pointer.
+ * COPY_FILE_SPLICE is an opt-in flag for cross-fs copy (e.g. from nfsd),
+ * so do not compare src<->dst filesystems in this case.
+ */
+static int copy_file_fs_cmp(struct file *f_in, struct file *f_out,
+			    unsigned int flags)
+{
+	if (flags & COPY_FILE_SPLICE)
+		return FS_COPY_SPLICE;
+	else if (f_out->f_op->copy_file_range &&
+		 f_out->f_op->copy_file_range == f_in->f_op->copy_file_range)
+		return FS_COPY_SAME_FS;
+	else if (file_inode(f_in)->i_sb == file_inode(f_out)->i_sb)
+		return FS_COPY_SAME_SB;
+	else
+		return FS_COPY_CROSS_FS;
+}
+
 /*
  * Performs necessary checks before doing a file copy
  *
@@ -1481,7 +1510,7 @@ COMPAT_SYSCALL_DEFINE4(sendfile64, int, out_fd, int, in_fd,
  */
 static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 				    struct file *file_out, loff_t pos_out,
-				    size_t *req_count, unsigned int flags)
+				    size_t *req_count, int fscmp)
 {
 	struct inode *inode_in = file_inode(file_in);
 	struct inode *inode_out = file_inode(file_out);
@@ -1498,18 +1527,13 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	 * a file of the wrong filesystem type to filesystem driver can result
 	 * in an attempt to dereference the wrong type of ->private_data, so
 	 * avoid doing that until we really have a good reason.
-	 *
-	 * nfs and cifs define several different file_system_type structures
-	 * and several different sets of file_operations, but they all end up
-	 * using the same ->copy_file_range() function pointer.
 	 */
-	if (flags & COPY_FILE_SPLICE) {
+	if (fscmp == FS_COPY_SPLICE) {
 		/* cross sb splice is allowed */
 	} else if (file_out->f_op->copy_file_range) {
-		if (file_in->f_op->copy_file_range !=
-		    file_out->f_op->copy_file_range)
+		if (fscmp != FS_COPY_SAME_FS)
 			return -EXDEV;
-	} else if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb) {
+	} else if (fscmp != FS_COPY_SAME_SB) {
 		return -EXDEV;
 	}
 
@@ -1556,13 +1580,13 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 {
 	ssize_t ret;
 	bool splice = flags & COPY_FILE_SPLICE;
-	bool samesb = file_inode(file_in)->i_sb == file_inode(file_out)->i_sb;
+	int fscmp = copy_file_fs_cmp(file_in, file_out, flags);
 
 	if (flags & ~COPY_FILE_SPLICE)
 		return -EINVAL;
 
 	ret = generic_copy_file_checks(file_in, pos_in, file_out, pos_out, &len,
-				       flags);
+				       fscmp);
 	if (unlikely(ret))
 		return ret;
 
@@ -1591,19 +1615,27 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	 * same sb using clone, but for filesystems where both clone and copy
 	 * are supported (e.g. nfs,cifs), we only call the copy method.
 	 */
-	if (!splice && file_out->f_op->copy_file_range) {
+	switch (fscmp) {
+	case FS_COPY_SPLICE:
+		break;
+	case FS_COPY_SAME_FS:
 		ret = file_out->f_op->copy_file_range(file_in, pos_in,
 						      file_out, pos_out,
 						      len, flags);
-	} else if (!splice && file_in->f_op->remap_file_range && samesb) {
-		ret = file_in->f_op->remap_file_range(file_in, pos_in,
-				file_out, pos_out, len, REMAP_FILE_CAN_SHORTEN);
-		/* fallback to splice */
+		break;
+	case FS_COPY_SAME_SB:
+		if (file_in->f_op->remap_file_range)
+			ret = file_in->f_op->remap_file_range(file_in, pos_in,
+							file_out, pos_out, len,
+							REMAP_FILE_CAN_SHORTEN);
+		/* Fallback to splice for same sb copy for backward compat */
 		if (ret <= 0)
 			splice = true;
-	} else if (samesb) {
-		/* Fallback to splice for same sb copy for backward compat */
-		splice = true;
+		break;
+	case FS_COPY_CROSS_FS:
+		/* This should have failed in generic_copy_file_checks() */
+		ret = -EXDEV;
+		break;
 	}
 
 	file_end_write(file_out);
-- 
2.54.0

[RFC][PATCH 2/5] fs: add support for copy file range from another fs

[Amir Goldstein]

Add a declarative vfs flag FOP_CROSS_FS_COPY for filesystems that may
be able copy from/to other fs and implement the copy from another fs,
which is the easy side.

When filesystem is copying from another fs, it will typically call vfs
read helpers to read from the other fs, so avoid the double accounting
and double fsnotify read hooks in vfs_copy_file_range().

Implementation of copy to another fs is a bit more subtle and will be
handled by a followup patch.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/read_write.c    | 18 ++++++++++++++----
 include/linux/fs.h |  2 ++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index d1565a56d089d..78596d9c89119 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1477,6 +1477,7 @@ enum {
 	FS_COPY_SAME_SB = 1,
 	FS_COPY_SAME_FS = 2,
 	FS_COPY_CROSS_FS = 3,
+	FS_COPY_FROM_OTHER_FS = 4,
 };
 
 /*
@@ -1495,6 +1496,8 @@ static int copy_file_fs_cmp(struct file *f_in, struct file *f_out,
 	else if (f_out->f_op->copy_file_range &&
 		 f_out->f_op->copy_file_range == f_in->f_op->copy_file_range)
 		return FS_COPY_SAME_FS;
+	else if (f_out->f_op->fop_flags & FOP_CROSS_FS_COPY)
+		return FS_COPY_FROM_OTHER_FS;
 	else if (file_inode(f_in)->i_sb == file_inode(f_out)->i_sb)
 		return FS_COPY_SAME_SB;
 	else
@@ -1526,10 +1529,13 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	 * We allow some filesystems to handle cross sb copy, but passing
 	 * a file of the wrong filesystem type to filesystem driver can result
 	 * in an attempt to dereference the wrong type of ->private_data, so
-	 * avoid doing that until we really have a good reason.
+	 * avoid doing that unless at least one of the filesystems declares
+	 * cross fstype copy support with the FOP_CROSS_FS_COPY flag.
 	 */
 	if (fscmp == FS_COPY_SPLICE) {
 		/* cross sb splice is allowed */
+	} else if (fscmp == FS_COPY_FROM_OTHER_FS) {
+		/* Copy from other fs is allowed */
 	} else if (file_out->f_op->copy_file_range) {
 		if (fscmp != FS_COPY_SAME_FS)
 			return -EXDEV;
@@ -1619,6 +1625,7 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	case FS_COPY_SPLICE:
 		break;
 	case FS_COPY_SAME_FS:
+	case FS_COPY_FROM_OTHER_FS:
 		ret = file_out->f_op->copy_file_range(file_in, pos_in,
 						      file_out, pos_out,
 						      len, flags);
@@ -1665,13 +1672,16 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out, len, 0);
 done:
 	if (ret > 0) {
-		fsnotify_access(file_in);
-		add_rchar(current, ret);
+		if (fscmp != FS_COPY_FROM_OTHER_FS) {
+			fsnotify_access(file_in);
+			add_rchar(current, ret);
+		}
 		fsnotify_modify(file_out);
 		add_wchar(current, ret);
 	}
 
-	inc_syscr(current);
+	if (fscmp != FS_COPY_FROM_OTHER_FS)
+		inc_syscr(current);
 	inc_syscw(current);
 
 	return ret;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d10897b3a1e35..c7573e89557b9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1980,6 +1980,8 @@ struct file_operations {
 #define FOP_ASYNC_LOCK		((__force fop_flags_t)(1 << 6))
 /* File system supports uncached read/write buffered IO */
 #define FOP_DONTCACHE		((__force fop_flags_t)(1 << 7))
+/* Supports copy_file_range() to/from other fs types */
+#define FOP_CROSS_FS_COPY	((__force fop_flags_t)(1 << 8))
 
 /* Wrap a directory iterator that needs exclusive inode access */
 int wrap_directory_iterator(struct file *, struct dir_context *,
-- 
2.54.0

[RFC][PATCH 3/5] fs: add support for copy file range to another fs

[Amir Goldstein]

Implement copy to other fs in vfs_copy_file_range() -
vfs must call the operation on the source filesystem and the source
filesystem (e.g. overlayfs) will typically call vfs helpers to write
to the other fs, so avoid taking freeze protection and fsnotify write
hooks in vfs_copy_file_range().

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/read_write.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/fs/read_write.c b/fs/read_write.c
index 78596d9c89119..846db21ac05ca 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1478,6 +1478,7 @@ enum {
 	FS_COPY_SAME_FS = 2,
 	FS_COPY_CROSS_FS = 3,
 	FS_COPY_FROM_OTHER_FS = 4,
+	FS_COPY_TO_OTHER_FS = 5,
 };
 
 /*
@@ -1498,6 +1499,8 @@ static int copy_file_fs_cmp(struct file *f_in, struct file *f_out,
 		return FS_COPY_SAME_FS;
 	else if (f_out->f_op->fop_flags & FOP_CROSS_FS_COPY)
 		return FS_COPY_FROM_OTHER_FS;
+	else if (f_in->f_op->fop_flags & FOP_CROSS_FS_COPY)
+		return FS_COPY_TO_OTHER_FS;
 	else if (file_inode(f_in)->i_sb == file_inode(f_out)->i_sb)
 		return FS_COPY_SAME_SB;
 	else
@@ -1534,8 +1537,9 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	 */
 	if (fscmp == FS_COPY_SPLICE) {
 		/* cross sb splice is allowed */
-	} else if (fscmp == FS_COPY_FROM_OTHER_FS) {
-		/* Copy from other fs is allowed */
+	} else if (fscmp == FS_COPY_FROM_OTHER_FS ||
+		   fscmp == FS_COPY_TO_OTHER_FS) {
+		/* Copy from/to other fs is allowed */
 	} else if (file_out->f_op->copy_file_range) {
 		if (fscmp != FS_COPY_SAME_FS)
 			return -EXDEV;
@@ -1587,6 +1591,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	ssize_t ret;
 	bool splice = flags & COPY_FILE_SPLICE;
 	int fscmp = copy_file_fs_cmp(file_in, file_out, flags);
+	const struct file_operations *fop =
+		(fscmp == FS_COPY_TO_OTHER_FS) ? file_in->f_op : file_out->f_op;
 
 	if (flags & ~COPY_FILE_SPLICE)
 		return -EINVAL;
@@ -1611,30 +1617,33 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 	 * Make sure return value doesn't overflow in 32bit compat mode.  Also
 	 * limit the size for all cases except when calling ->copy_file_range().
 	 */
-	if (splice || !file_out->f_op->copy_file_range || in_compat_syscall())
+	if (splice || !fop->copy_file_range || in_compat_syscall())
 		len = min_t(size_t, MAX_RW_COUNT, len);
 
-	file_start_write(file_out);
+	if (fscmp != FS_COPY_TO_OTHER_FS)
+		file_start_write(file_out);
 
 	/*
 	 * Cloning is supported by more file systems, so we implement copy on
 	 * same sb using clone, but for filesystems where both clone and copy
 	 * are supported (e.g. nfs,cifs), we only call the copy method.
+	 * Cross-fs copy (FROM_OTHER_FS / TO_OTHER_FS) is handled by whichever
+	 * side declared FOP_CROSS_FS_COPY.
 	 */
 	switch (fscmp) {
 	case FS_COPY_SPLICE:
 		break;
 	case FS_COPY_SAME_FS:
+	case FS_COPY_TO_OTHER_FS:
 	case FS_COPY_FROM_OTHER_FS:
-		ret = file_out->f_op->copy_file_range(file_in, pos_in,
-						      file_out, pos_out,
-						      len, flags);
+		ret = fop->copy_file_range(file_in, pos_in, file_out, pos_out,
+					   len, flags);
 		break;
 	case FS_COPY_SAME_SB:
-		if (file_in->f_op->remap_file_range)
-			ret = file_in->f_op->remap_file_range(file_in, pos_in,
-							file_out, pos_out, len,
-							REMAP_FILE_CAN_SHORTEN);
+		if (fop->remap_file_range)
+			ret = fop->remap_file_range(file_in, pos_in,
+						    file_out, pos_out, len,
+						    REMAP_FILE_CAN_SHORTEN);
 		/* Fallback to splice for same sb copy for backward compat */
 		if (ret <= 0)
 			splice = true;
@@ -1645,7 +1654,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 		break;
 	}
 
-	file_end_write(file_out);
+	if (fscmp != FS_COPY_TO_OTHER_FS)
+		file_end_write(file_out);
 
 	if (!splice)
 		goto done;
@@ -1676,13 +1686,16 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 			fsnotify_access(file_in);
 			add_rchar(current, ret);
 		}
-		fsnotify_modify(file_out);
-		add_wchar(current, ret);
+		if (fscmp != FS_COPY_TO_OTHER_FS) {
+			fsnotify_modify(file_out);
+			add_wchar(current, ret);
+		}
 	}
 
 	if (fscmp != FS_COPY_FROM_OTHER_FS)
 		inc_syscr(current);
-	inc_syscw(current);
+	if (fscmp != FS_COPY_TO_OTHER_FS)
+		inc_syscw(current);
 
 	return ret;
 }
-- 
2.54.0

[RFC][PATCH 4/5] ovl: add support for copy file range from another fs

[Amir Goldstein]

It is technically possible for overlayfs to copy_file_range() from
the base fs of the real layer.

Set the vfs flag FOP_CROSS_FS_COPY and implement copy from another fs.

Implementation of copy to another fs is a bit more subtle and will be
handled by a followup patch.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c      | 22 ++++++++++++++++++++++
 fs/overlayfs/ovl_entry.h |  7 ++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index f3d97eb146e85..169a48de565e9 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -533,6 +533,13 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 	struct file *realfile_in, *realfile_out;
 	loff_t ret;
 
+	/*
+	 * Overlayfs may be able to copy to a non overlayfs dst which is on the
+	 * same sb or same fs type as real file_in fs, but not implemented yet.
+	 */
+	if (WARN_ON_ONCE(!is_ovl_fs(inode_out->i_sb)))
+		return -EXDEV;
+
 	inode_lock(inode_out);
 	if (op != OVL_DEDUPE) {
 		/* Update mode */
@@ -547,6 +554,19 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 	if (IS_ERR(realfile_out))
 		goto out_unlock;
 
+	/*
+	 * Overlayfs may be able to copy from a non overlayfs src which is on
+	 * the same sb or same fs type as upper fs.
+	 */
+	if (unlikely(!is_ovl_fs(inode_in->i_sb))) {
+		if (WARN_ON_ONCE(op != OVL_COPY)) {
+			ret = -EXDEV;
+			goto out_unlock;
+		}
+		realfile_in = file_in;
+		goto do_copy;
+	}
+
 	realfile_in = ovl_real_file(file_in);
 	ret = PTR_ERR(realfile_in);
 	if (IS_ERR(realfile_in))
@@ -565,6 +585,7 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 		}
 	}
 
+do_copy:
 	with_ovl_creds(inode_out->i_sb) {
 		switch (op) {
 		case OVL_COPY:
@@ -660,6 +681,7 @@ const struct file_operations ovl_file_operations = {
 	.splice_read    = ovl_splice_read,
 	.splice_write   = ovl_splice_write,
 
+	.fop_flags		= FOP_CROSS_FS_COPY,
 	.copy_file_range	= ovl_copy_file_range,
 	.remap_file_range	= ovl_remap_file_range,
 	.setlease		= generic_setlease,
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 80cad4ea96a3e..8a1862cf87d78 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -112,10 +112,15 @@ static inline struct mnt_idmap *ovl_upper_mnt_idmap(struct ovl_fs *ofs)
 
 extern struct file_system_type ovl_fs_type;
 
+static inline bool is_ovl_fs(const struct super_block *sb)
+{
+	return sb->s_type == &ovl_fs_type;
+}
+
 static inline struct ovl_fs *OVL_FS(struct super_block *sb)
 {
 	if (IS_ENABLED(CONFIG_OVERLAY_FS_DEBUG))
-		WARN_ON_ONCE(sb->s_type != &ovl_fs_type);
+		WARN_ON_ONCE(!is_ovl_fs(sb));
 
 	return (struct ovl_fs *)sb->s_fs_info;
 }
-- 
2.54.0

[RFC][PATCH 5/5] ovl: add support for copy file range to another fs

[Amir Goldstein]

It is technically possible for overlayfs to copy_file_range() to
the base fs of the real layer.

Implementation of copy to other fs is a bit more subtle, because
we must not take the other fs locks in overlayfs code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/file.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 169a48de565e9..cc52ccc35961f 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -535,10 +535,23 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 
 	/*
 	 * Overlayfs may be able to copy to a non overlayfs dst which is on the
-	 * same sb or same fs type as real file_in fs, but not implemented yet.
+	 * same sb or same fs type as real file_in fs, but we must avoid taking
+	 * the locks on the foreign fs.
 	 */
-	if (WARN_ON_ONCE(!is_ovl_fs(inode_out->i_sb)))
-		return -EXDEV;
+	if (unlikely(!is_ovl_fs(inode_out->i_sb))) {
+		if (WARN_ON_ONCE(op != OVL_COPY) ||
+		    WARN_ON_ONCE(!is_ovl_fs(file_inode(file_in)->i_sb)))
+			return -EXDEV;
+
+		realfile_in = ovl_real_file(file_in);
+		if (IS_ERR(realfile_in))
+			return PTR_ERR(realfile_in);
+
+		with_ovl_creds(file_inode(file_in)->i_sb)
+			return vfs_copy_file_range(realfile_in, pos_in,
+						   file_out, pos_out, len,
+						   flags);
+	}
 
 	inode_lock(inode_out);
 	if (op != OVL_DEDUPE) {
-- 
2.54.0

Re: [RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Daan De Meyer]

Hi Amir,

I tested the series on top of f6cde11eb46e and cherry-picked these
selftests:

https://github.com/daandemeyer/vfs/commit/0b0e47d408311fe2b4a3e1711656f836b56e10f4

All four tests failed.

The accounting test reports two reads and writes, and 128 KiB
accounted in each direction, for one 64 KiB copy. The outer and
recursive backing-file VFS calls both account the operation.

The credential test fails with EPERM. The explicit source check uses
the source mount credentials, but the recursive VFS call checks the
real source again while the destination mount credentials are active.

The permission-order test times out because the real source
FAN_ACCESS_PERM event happens after the destination inode has been
locked. The process handling the event cannot update the destination
while the copy is waiting for its response.

Finally, denying that permission event still clears the destination's
setuid and setgid bits. ovl_copyfile() removes them before checking
permission on the real source.

It looks like the root cause is that OverlayFS-to-OverlayFS is
classified as FS_COPY_SAME_FS before the FOP_CROSS_FS_COPY checks.
That leaves it on the existing recursive VFS path, so these issues
remain even with the series applied.

My (admittedly much larger) patch set does pass all of these selftests
by moving most of
the authorization checks out of overlayfs and into vfs and can be found here:
https://github.com/linux-fsdevel/vfs/compare/vfs.base...daandemeyer:vfs:push-zolnnszvmkry
for those interested in taking a look. Happy to post it directly to
the list as an RFC if that's
preferred over a github link. That patch set also lays a foundation
for cross sb copy_file_range()
but I opted to not post those patches yet so we can figure out how we
want to deal with the above
issues first.

Cheers,
Daan


On Thu, 23 Jul 2026 at 13:48, Amir Goldstein <amir73il@gmail.com> wrote:
>
> Hi all,
>
> Daan De Meyer requested a way to do copy_file_range() from overlayfs
> to/from another fs, where if that fs is the base fs of overlayfs layer
> the copy could be an efficient clone.
>
> TBH, he also requested support also for cross-fs clone_file_range(),
> but I am not happy about providing that.
>
> copy_file_range() already supports cross-sb efficient copy (e.g. with
> NFS/SMB server-side copy).
>
> The current limitation of staying on the same fstype is mainly an
> internal technical limitation (choosing the right f_op), which is
> something that this RFC is trying to address.
>
> The overlayfs implementation itself is pretty trivial.
>
> Thanks,
> Amir.
>
>
> Amir Goldstein (5):
>   fs: clarify cross-sb copy_file_range() code
>   fs: add support for copy file range from another fs
>   fs: add support for copy file range to another fs
>   ovl: add support for copy file range from another fs
>   ovl: add support for copy file range to another fs
>
>  fs/overlayfs/file.c      |  35 ++++++++++++
>  fs/overlayfs/ovl_entry.h |   7 ++-
>  fs/read_write.c          | 119 ++++++++++++++++++++++++++++-----------
>  include/linux/fs.h       |   2 +
>  4 files changed, 130 insertions(+), 33 deletions(-)
>
> --
> 2.54.0
>

Re: [RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Amir Goldstein]

On Thu, Jul 23, 2026 at 3:05 PM Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
>
> Hi Amir,
>
> I tested the series on top of f6cde11eb46e and cherry-picked these
> selftests:
>
> https://github.com/daandemeyer/vfs/commit/0b0e47d408311fe2b4a3e1711656f836b56e10f4
>
> All four tests failed.
>
> The accounting test reports two reads and writes, and 128 KiB
> accounted in each direction, for one 64 KiB copy. The outer and
> recursive backing-file VFS calls both account the operation.
>
> The credential test fails with EPERM. The explicit source check uses
> the source mount credentials, but the recursive VFS call checks the
> real source again while the destination mount credentials are active.
>
> The permission-order test times out because the real source
> FAN_ACCESS_PERM event happens after the destination inode has been
> locked. The process handling the event cannot update the destination
> while the copy is waiting for its response.
>
> Finally, denying that permission event still clears the destination's
> setuid and setgid bits. ovl_copyfile() removes them before checking
> permission on the real source.
>
> It looks like the root cause is that OverlayFS-to-OverlayFS is
> classified as FS_COPY_SAME_FS before the FOP_CROSS_FS_COPY checks.
> That leaves it on the existing recursive VFS path, so these issues
> remain even with the series applied.

I am baffled by this statement.

As far as I can see, OverlayFS-to-OverlayFS, which is classified
as FS_COPY_SAME_FS should not have changed behavior at all.
That was my intention at least, maybe I have a bug (?).

If I am right and behavior did not change then what are you saying?
That OverlayFS-to-OverlayFS copy_file_range() in upstream has issues?
Are those issues unique to copy_file_range() or also exist for clone
and splice and regular read/write?

Note the overlayfs model - do every operation on both overlayfs mount
(with user creds) and on backing mount (with ovl mounter creds).
Taking this model into consideration, what in your opinion is broken w.r.t
upstream [1] behavior of OverlayFS-to-OverlayFS copy?

Thanks,
Amir.

[1] upstream + this fix
https://lore.kernel.org/linux-unionfs/20260712122421.203113-1-amir73il@gmail.com/

Re: [RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Daan De Meyer]

> As far as I can see, OverlayFS-to-OverlayFS, which is classified
> as FS_COPY_SAME_FS should not have changed behavior at all.
> That was my intention at least, maybe I have a bug (?).

I am sorry, I forgot to mention that all of these issues are issues
with upstream overlayfs,
not with your patchset. I was trying to make the argument that to
solve the issues that are
already in upstream overlayfs and to introduce cross sb
copy_file_range() we need to move
away from recursively calling vfs_copy_file_range() within overlayfs
(and anywhere else really)
as its internals are really not intended for being called recursively.

`ovl_copyfile()` locks the overlay destination, removes privileges,
and then calls `vfs_copy_file_range()`
on the backing files under the destination OverlayFS mount credentials.

This has three observable consequences:

1. The recursive VFS call accounts one userspace copy twice.
2. When the files are on different OverlayFS mounts, the backing
source permission check uses the
destination mount credentials rather than the source mount credentials.
3. Backing permission hooks run while the overlay destination inode is
locked and after privileges
have been removed. A permission listener that accesses the destination
can deadlock, and a denied
backing check leaves the destination setid bits cleared.

Clone and dedupe use the same `ovl_copyfile()` helper and share the
credential and locking issue;
clone also shares the killpriv issue. Splice does not have the same
mixed-credential problem because
its source and destination operations are separate.

If anyone else thinks we should fix these pre-existing issues, then I
don't think recursively calling into
vfs_copy_file_range() is the way to go. If they're deemed not
important, then recursively calling into
vfs_copy_file_range() is probably fine.

Cheers,

Daan

On Thu, 23 Jul 2026 at 15:36, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Jul 23, 2026 at 3:05 PM Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
> >
> > Hi Amir,
> >
> > I tested the series on top of f6cde11eb46e and cherry-picked these
> > selftests:
> >
> > https://github.com/daandemeyer/vfs/commit/0b0e47d408311fe2b4a3e1711656f836b56e10f4
> >
> > All four tests failed.
> >
> > The accounting test reports two reads and writes, and 128 KiB
> > accounted in each direction, for one 64 KiB copy. The outer and
> > recursive backing-file VFS calls both account the operation.
> >
> > The credential test fails with EPERM. The explicit source check uses
> > the source mount credentials, but the recursive VFS call checks the
> > real source again while the destination mount credentials are active.
> >
> > The permission-order test times out because the real source
> > FAN_ACCESS_PERM event happens after the destination inode has been
> > locked. The process handling the event cannot update the destination
> > while the copy is waiting for its response.
> >
> > Finally, denying that permission event still clears the destination's
> > setuid and setgid bits. ovl_copyfile() removes them before checking
> > permission on the real source.
> >
> > It looks like the root cause is that OverlayFS-to-OverlayFS is
> > classified as FS_COPY_SAME_FS before the FOP_CROSS_FS_COPY checks.
> > That leaves it on the existing recursive VFS path, so these issues
> > remain even with the series applied.
>
> I am baffled by this statement.
>
> As far as I can see, OverlayFS-to-OverlayFS, which is classified
> as FS_COPY_SAME_FS should not have changed behavior at all.
> That was my intention at least, maybe I have a bug (?).
>
> If I am right and behavior did not change then what are you saying?
> That OverlayFS-to-OverlayFS copy_file_range() in upstream has issues?
> Are those issues unique to copy_file_range() or also exist for clone
> and splice and regular read/write?
>
> Note the overlayfs model - do every operation on both overlayfs mount
> (with user creds) and on backing mount (with ovl mounter creds).
> Taking this model into consideration, what in your opinion is broken w.r.t
> upstream [1] behavior of OverlayFS-to-OverlayFS copy?
>
> Thanks,
> Amir.
>
> [1] upstream + this fix
> https://lore.kernel.org/linux-unionfs/20260712122421.203113-1-amir73il@gmail.com/

Re: [RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Amir Goldstein]

On Thu, Jul 23, 2026 at 3:54 PM Daan De Meyer <daan.j.demeyer@gmail.com> wrote:
>
> > As far as I can see, OverlayFS-to-OverlayFS, which is classified
> > as FS_COPY_SAME_FS should not have changed behavior at all.
> > That was my intention at least, maybe I have a bug (?).
>
> I am sorry, I forgot to mention that all of these issues are issues
> with upstream overlayfs,
> not with your patchset. I was trying to make the argument that to
> solve the issues that are
> already in upstream overlayfs and to introduce cross sb
> copy_file_range() we need to move
> away from recursively calling vfs_copy_file_range() within overlayfs
> (and anywhere else really)
> as its internals are really not intended for being called recursively.
>
> `ovl_copyfile()` locks the overlay destination, removes privileges,
> and then calls `vfs_copy_file_range()`
> on the backing files under the destination OverlayFS mount credentials.

Yes absolutely by design

>
> This has three observable consequences:
>
> 1. The recursive VFS call accounts one userspace copy twice.

I think that is fine or at least not so bad.
same "problem" exist for overlayfs read/write and has been that way
forever. nobody complained.

> 2. When the files are on different OverlayFS mounts, the backing
> source permission check uses the
> destination mount credentials rather than the source mount credentials.

You mean that after the fix [1] it is *also* checked by destination mount
creds after being checked with source mount creds.

There is no security risk here, so I think this is something we should
leave as a known and documented issue, because solving that would
be as complicated as what you posted and I don't want to go there.

> 3. Backing permission hooks run while the overlay destination inode is
> locked and after privileges
> have been removed. A permission listener that accesses the destination
> can deadlock,

There are endless variants to these potential deadlocks with permission
events. This was discussed recently on this thread:
https://lore.kernel.org/linux-fsdevel/xycqm3qrhjg3uplpmozwo7nterl5tnuarghowmh3h3mv3thh6u@apagf4kqlkg6/

> and a denied
> backing check leaves the destination setid bits cleared.

Nothing new here - not even a bug or a problem IMO.
The state of privs after a write that MAY have started and returned an
error is undefined.
Always been this way in all the write APIs.

IOW, all the problems you listed are overlayfs behavior by design,
which AFAIK have been like this forever.

Do you have any reports on new issues specific to this patch set and
the new overlayfs to/from basefs copy?

Do you know of any real world workloads that are significantly impacted
by the existing upstream issues that you listed?

Thanks,
Amir.

Re: [RFC][PATCH 0/5] Proposal for Cross-fs copy_file_range()

[Gao Xiang]

Hi Amir,

On Thu, Jul 23, 2026 at 01:48:43PM +0200, Amir Goldstein wrote:
> Hi all,
> 
> Daan De Meyer requested a way to do copy_file_range() from overlayfs
> to/from another fs, where if that fs is the base fs of overlayfs layer
> the copy could be an efficient clone.
> 
> TBH, he also requested support also for cross-fs clone_file_range(),
> but I am not happy about providing that.
> 
> copy_file_range() already supports cross-sb efficient copy (e.g. with
> NFS/SMB server-side copy).

I once had a long-term goal of supporting copy_file_range() between
overlayfs and EROFS for file-backed mounts, as long as the EROFS inodes
are plain (i.e. uncompressed). Such cross-fs copies could be decomposed
into one or more sub-copies from the EROFS backing file to the
overlayfs upper file, so the underlying backing filesystem can perform
the actual data transfer. If the backing filesystem supports reflinks,
those operations could even be implemented as efficient reflinks.

This would greatly optimize overlayfs copy-up for file-backed EROFS
mounts.

I think this should be feasible, although I haven't had time to work
on it yet. If clean interfaces in this area make that easier, I'd be
very happy to make use of them and enable this later.

Thanks,
Gao Xiang

> 
> The current limitation of staying on the same fstype is mainly an
> internal technical limitation (choosing the right f_op), which is
> something that this RFC is trying to address.
> 
> The overlayfs implementation itself is pretty trivial.
> 
> Thanks,
> Amir.
> 
> 
> Amir Goldstein (5):
>   fs: clarify cross-sb copy_file_range() code
>   fs: add support for copy file range from another fs
>   fs: add support for copy file range to another fs
>   ovl: add support for copy file range from another fs
>   ovl: add support for copy file range to another fs
> 
>  fs/overlayfs/file.c      |  35 ++++++++++++
>  fs/overlayfs/ovl_entry.h |   7 ++-
>  fs/read_write.c          | 119 ++++++++++++++++++++++++++++-----------
>  include/linux/fs.h       |   2 +
>  4 files changed, 130 insertions(+), 33 deletions(-)
> 
> -- 
> 2.54.0
>