CEPH-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Luis Henriques <lhenriques@suse.de>
To: Amir Goldstein <amir73il@gmail.com>,
	Jeff Layton <jlayton@kernel.org>,
	Steve French <sfrench@samba.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Dave Chinner <dchinner@redhat.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Ian Lance Taylor <iant@google.com>,
	Luis Lozano <llozano@chromium.org>
Cc: ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
	Luis Henriques <lhenriques@suse.de>
Subject: [PATCH v3] vfs: fix copy_file_range regression in cross-fs copies
Date: Wed, 17 Feb 2021 17:26:54 +0000
Message-ID: <20210217172654.22519-1-lhenriques@suse.de> (raw)
In-Reply-To: <CAOQ4uxii=7KUKv1w32VbjkwS+Z1a0ge0gezNzpn_BiY6MFWkpA@mail.gmail.com>

A regression has been reported by Nicolas Boichat, found while using the
copy_file_range syscall to copy a tracefs file.  Before commit
5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices") the
kernel would return -EXDEV to userspace when trying to copy a file across
different filesystems.  After this commit, the syscall doesn't fail anymore
and instead returns zero (zero bytes copied), as this file's content is
generated on-the-fly and thus reports a size of zero.

This patch restores some cross-filesystems copy restrictions that existed
prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
devices").  It also introduces a flag (COPY_FILE_SPLICE) that can be used
by filesystems calling directly into the vfs copy_file_range to override
these restrictions.  Right now, only NFS needs to set this flag.

Fixes: 5dae222a5ff0 ("vfs: allow copy_file_range to copy across devices")
Link: https://lore.kernel.org/linux-fsdevel/20210212044405.4120619-1-drinkcat@chromium.org/
Link: https://lore.kernel.org/linux-fsdevel/CANMq1KDZuxir2LM5jOTm0xx+BnvW=ZmpsG47CyHFJwnw7zSX6Q@mail.gmail.com/
Link: https://lore.kernel.org/linux-fsdevel/20210126135012.1.If45b7cdc3ff707bc1efa17f5366057d60603c45f@changeid/
Reported-by: Nicolas Boichat <drinkcat@chromium.org>
Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
Ok, I've tried to address all the issues and comments.  Hopefully this v3
is a bit closer to the final fix.

Changes since v2
- do all the required checks earlier, in generic_copy_file_checks(),
  adding new checks for ->remap_file_range
- new COPY_FILE_SPLICE flag
- don't remove filesystem's fallback to generic_copy_file_range()
- updated commit changelog (and subject)
Changes since v1 (after Amir review)
- restored do_copy_file_range() helper
- return -EOPNOTSUPP if fs doesn't implement CFR
- updated commit description

 fs/nfsd/vfs.c      |  3 ++-
 fs/read_write.c    | 44 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/fs.h |  7 +++++++
 3 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 04937e51de56..14e55822c223 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -578,7 +578,8 @@ ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst,
 	 * limit like this and pipeline multiple COPY requests.
 	 */
 	count = min_t(u64, count, 1 << 22);
-	return vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0);
+	return vfs_copy_file_range(src, src_pos, dst, dst_pos, count,
+				   COPY_FILE_SPLICE);
 }
 
 __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp,
diff --git a/fs/read_write.c b/fs/read_write.c
index 75f764b43418..40a16003fb05 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1410,6 +1410,33 @@ static ssize_t do_copy_file_range(struct file *file_in, loff_t pos_in,
 				       flags);
 }
 
+/*
+ * This helper function checks whether copy_file_range can actually be used,
+ * depending on the source and destination filesystems being the same.
+ *
+ * In-kernel callers may set COPY_FILE_SPLICE to override these checks.
+ */
+static int fops_copy_file_checks(struct file *file_in, struct file *file_out,
+				 unsigned int flags)
+{
+	if (WARN_ON_ONCE(flags & ~COPY_FILE_SPLICE))
+		return -EINVAL;
+
+	if (flags & COPY_FILE_SPLICE)
+		return 0;
+	/*
+	 * We got here from userspace, so forbid copies if copy_file_range isn't
+	 * implemented or if we're doing a cross-fs copy.
+	 */
+	if (!file_out->f_op->copy_file_range)
+		return -EOPNOTSUPP;
+	else if (file_out->f_op->copy_file_range !=
+		 file_in->f_op->copy_file_range)
+		return -EXDEV;
+
+	return 0;
+}
+
 /*
  * Performs necessary checks before doing a file copy
  *
@@ -1427,6 +1454,14 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	loff_t size_in;
 	int ret;
 
+	/* Only check f_ops if we're not trying to clone */
+	if (!file_in->f_op->remap_file_range ||
+	    (file_inode(file_in)->i_sb == file_inode(file_out)->i_sb)) {
+		ret = fops_copy_file_checks(file_in, file_out, flags);
+		if (ret)
+			return ret;
+	}
+
 	ret = generic_file_rw_checks(file_in, file_out);
 	if (ret)
 		return ret;
@@ -1474,9 +1509,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 {
 	ssize_t ret;
 
-	if (flags != 0)
-		return -EINVAL;
-
 	ret = generic_copy_file_checks(file_in, pos_in, file_out, pos_out, &len,
 				       flags);
 	if (unlikely(ret))
@@ -1511,6 +1543,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 			ret = cloned;
 			goto done;
 		}
+		ret = fops_copy_file_checks(file_in, file_out, flags);
+		if (ret)
+			return ret;
 	}
 
 	ret = do_copy_file_range(file_in, pos_in, file_out, pos_out, len,
@@ -1543,6 +1578,9 @@ SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
 	struct fd f_out;
 	ssize_t ret = -EBADF;
 
+	if (flags != 0)
+		return -EINVAL;
+
 	f_in = fdget(fd_in);
 	if (!f_in.file)
 		goto out2;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd47deea7c17..6f604926d955 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1815,6 +1815,13 @@ struct dir_context {
  */
 #define REMAP_FILE_ADVISORY		(REMAP_FILE_CAN_SHORTEN)
 
+/*
+ * This flag control the behavior of copy_file_range from internal (kernel)
+ * users.  It can be used to override the policy of forbidding copies when
+ * source and destination filesystems are different.
+ */
+#define COPY_FILE_SPLICE		(1 << 0)
+
 struct iov_iter;
 
 struct file_operations {

  reply index

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAOQ4uxiFGjdvX2-zh5o46pn7RZhvbGHH0wpzLPuPOom91FwWeQ@mail.gmail.com>
2021-02-15 15:43 ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Luis Henriques
2021-02-15 16:02   ` Trond Myklebust
2021-02-16  0:25     ` Steve French
2021-02-15 16:34   ` Amir Goldstein
2021-02-15 16:53     ` Trond Myklebust
2021-02-15 17:24       ` Amir Goldstein
2021-02-15 18:57         ` Trond Myklebust
2021-02-15 19:43           ` Amir Goldstein
2021-02-16 11:17             ` Luis Henriques
2021-02-16 11:28               ` gregkh
2021-02-16 12:01                 ` Luis Henriques
2021-02-16 12:08                   ` Greg KH
2021-02-16 13:51               ` Amir Goldstein
2021-02-16 16:42                 ` Luis Henriques
2021-02-16 17:44                   ` Amir Goldstein
2021-02-16 18:55                     ` Luis Henriques
2021-02-16 19:20                       ` Amir Goldstein
2021-02-16 19:27                         ` Anna Schumaker
2021-02-16 19:31                           ` Steve French
2021-02-16 19:40                             ` Amir Goldstein
2021-02-16 21:15                               ` Steve French
2021-02-17  8:08                                 ` Amir Goldstein
2021-02-17 17:26                                   ` Luis Henriques [this message]
2021-02-17 20:47                                     ` [PATCH v3] vfs: fix copy_file_range regression in cross-fs copies Amir Goldstein
2021-02-18  0:56                                     ` Nicolas Boichat
2021-02-18  5:32                                     ` Olga Kornievskaia
2021-02-18  6:47                                       ` Amir Goldstein
2021-02-18 16:28                                         ` Olga Kornievskaia
2021-02-18  7:43                                     ` Christoph Hellwig
2021-02-18  0:50                                   ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Andreas Dilger
2021-02-18  7:34                                     ` gregkh
2021-02-16 18:54                 ` Andreas Dilger
2021-02-17  4:45   ` Nicolas Boichat
2021-02-18  7:42   ` Christoph Hellwig
2021-02-18  9:10     ` Amir Goldstein
2021-02-18 10:29       ` Luis Henriques
2021-02-18 12:15         ` Luis Henriques
2021-02-18 12:49           ` Amir Goldstein
2021-02-18 14:36             ` [PATCH v4] vfs: fix copy_file_range regression in cross-fs copies Luis Henriques
2021-02-18 14:58               ` Amir Goldstein
2021-02-18 15:17                 ` [PATCH v5] " Luis Henriques
2021-02-18 15:53                   ` Amir Goldstein
2021-02-18 16:35                     ` Luis Henriques
2021-02-18 17:18                       ` [PATCH v6] " Luis Henriques
2021-02-19 21:18                         ` Olga Kornievskaia
2021-02-19 21:52                           ` Amir Goldstein
2021-02-21 19:58                           ` [PATCH v7] " Luis Henriques
2021-02-22  3:00                             ` Nicolas Boichat
2021-02-22 10:24                             ` [PATCH v8] " Luis Henriques
2021-02-22 10:46                               ` Amir Goldstein
2021-02-22 16:25                               ` dai.ngo
2021-02-23 10:32                                 ` Luis Henriques
2021-02-23 15:28                                   ` Amir Goldstein
2021-02-23 15:29                                   ` dai.ngo
2021-02-23 16:02                                     ` dai.ngo
2021-02-23 16:47                                       ` Amir Goldstein
2021-02-23 16:57                                         ` dai.ngo
     [not found]                                           ` <e3eed18b-fc7e-e687-608b-7f662017329c@oracle.com>
2021-02-23 17:33                                             ` Amir Goldstein
2021-02-24  0:13                                               ` dai.ngo
2021-02-23 17:56                                           ` Luis Henriques
2021-02-23 17:13                                       ` Olga Kornievskaia
2021-02-24  1:00                               ` Olga Kornievskaia
2021-02-24 10:23                                 ` Luis Henriques
2021-02-24 10:44                                   ` Nicolas Boichat
2021-04-09  5:23                                     ` Nicolas Boichat
2021-04-09 13:39                                       ` Luis Henriques
2021-04-09 13:50                                         ` Amir Goldstein
2021-04-23  4:40                                           ` Nicolas Boichat
2021-05-03  8:54                                             ` Luis Henriques
2021-02-24 14:23                               ` [PATCH] copy_file_range.2: Kernel v5.12 updates Luis Henriques
2021-02-24 16:10                                 ` Amir Goldstein
2021-02-25 10:21                                   ` Luis Henriques
2021-02-26 10:13                                     ` Alejandro Colomar (man-pages)
2021-02-26 10:34                                       ` Amir Goldstein
2021-02-26 11:15                                         ` Alejandro Colomar (man-pages)
2021-02-26 13:59                                           ` Jeff Layton
2021-02-26 21:26                                             ` Alejandro Colomar (man-pages)
2021-02-26 22:18                                   ` Alejandro Colomar (man-pages)
2021-02-27  5:41                                     ` Amir Goldstein
2021-02-27 12:20                                       ` Alejandro Colomar (man-pages)
2021-02-27 13:49                                         ` [RFC v2] copy_file_range.2: Update cross-filesystem support for 5.12 Alejandro Colomar
2021-02-27 16:00                                           ` Amir Goldstein
2021-02-27 23:08                                       ` [PATCH] copy_file_range.2: Kernel v5.12 updates Steve French
2021-02-28  7:35                                         ` Amir Goldstein
2021-02-28 22:25                                           ` Steve French
2021-03-01  6:18                                             ` Amir Goldstein
2021-03-01 14:41                                 ` [RFC v3] copy_file_range.2: Update cross-filesystem support for 5.12 Alejandro Colomar
2021-03-01 14:58                                   ` Amir Goldstein
2021-03-04  9:38                                 ` [RFC v4] " Alejandro Colomar
2021-03-04 17:13                                   ` Darrick J. Wong
2021-03-04 18:24                                     ` Alejandro Colomar (man-pages)
2021-03-04 23:50                                       ` Darrick J. Wong
2021-02-18 20:41       ` [PATCH v2] vfs: prevent copy_file_range to copy across devices Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210217172654.22519-1-lhenriques@suse.de \
    --to=lhenriques@suse.de \
    --cc=amir73il@gmail.com \
    --cc=anna.schumaker@netapp.com \
    --cc=ceph-devel@vger.kernel.org \
    --cc=darrick.wong@oracle.com \
    --cc=dchinner@redhat.com \
    --cc=drinkcat@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=iant@google.com \
    --cc=jlayton@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=llozano@chromium.org \
    --cc=miklos@szeredi.hu \
    --cc=samba-technical@lists.samba.org \
    --cc=sfrench@samba.org \
    --cc=trond.myklebust@hammerspace.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

CEPH-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/ceph-devel/0 ceph-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ceph-devel ceph-devel/ https://lore.kernel.org/ceph-devel \
		ceph-devel@vger.kernel.org
	public-inbox-index ceph-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.ceph-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git