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>,
	Andreas Dilger <adilger@dilger.ca>,
	Olga Kornievskaia <aglo@umich.edu>,
	Christoph Hellwig <hch@infradead.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 v8] vfs: fix copy_file_range regression in cross-fs copies
Date: Mon, 22 Feb 2021 10:24:56 +0000
Message-ID: <20210222102456.6692-1-lhenriques@suse.de> (raw)
In-Reply-To: <20210221195833.23828-1-lhenriques@suse.de>

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-filesystem copy restrictions that existed
prior to commit 5dae222a5ff0 ("vfs: allow copy_file_range to copy across
devices").  Filesystems are still allowed to fall-back to the VFS
generic_copy_file_range() implementation, but that has now to be done
explicitly.

nfsd is also modified to fall-back into generic_copy_file_range() in case
vfs_copy_file_range() fails with -EOPNOTSUPP or -EXDEV.

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>
---
Changes since v7
- set 'ret' to '-EOPNOTSUPP' before the clone 'if' statement so that the
  error returned is always related to the 'copy' operation
Changes since v6
- restored i_sb checks for the clone operation
Changes since v5
- check if ->copy_file_range is NULL before calling it
Changes since v4
- nfsd falls-back to generic_copy_file_range() only *if* it gets -EOPNOTSUPP
  or -EXDEV.
Changes since v3
- dropped the COPY_FILE_SPLICE flag
- kept the f_op's checks early in generic_copy_file_checks, implementing
  Amir's suggestions
- modified nfsd to use generic_copy_file_range()
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   |  8 +++++++-
 fs/read_write.c | 49 ++++++++++++++++++++++++-------------------------
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 04937e51de56..23dab0fa9087 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -568,6 +568,7 @@ __be32 nfsd4_clone_file_range(struct nfsd_file *nf_src, u64 src_pos,
 ssize_t nfsd_copy_file_range(struct file *src, u64 src_pos, struct file *dst,
 			     u64 dst_pos, u64 count)
 {
+	ssize_t ret;
 
 	/*
 	 * Limit copy to 4MB to prevent indefinitely blocking an nfsd
@@ -578,7 +579,12 @@ 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);
+	ret = vfs_copy_file_range(src, src_pos, dst, dst_pos, count, 0);
+
+	if (ret == -EOPNOTSUPP || ret == -EXDEV)
+		ret = generic_copy_file_range(src, src_pos, dst, dst_pos,
+					      count, 0);
+	return ret;
 }
 
 __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..5a26297fd410 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1388,28 +1388,6 @@ ssize_t generic_copy_file_range(struct file *file_in, loff_t pos_in,
 }
 EXPORT_SYMBOL(generic_copy_file_range);
 
-static ssize_t do_copy_file_range(struct file *file_in, loff_t pos_in,
-				  struct file *file_out, loff_t pos_out,
-				  size_t len, unsigned int flags)
-{
-	/*
-	 * Although we now allow filesystems to handle cross sb copy, 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.  NFS defines
-	 * several different file_system_type structures, but they all end up
-	 * using the same ->copy_file_range() function pointer.
-	 */
-	if (file_out->f_op->copy_file_range &&
-	    file_out->f_op->copy_file_range == file_in->f_op->copy_file_range)
-		return file_out->f_op->copy_file_range(file_in, pos_in,
-						       file_out, pos_out,
-						       len, flags);
-
-	return generic_copy_file_range(file_in, pos_in, file_out, pos_out, len,
-				       flags);
-}
-
 /*
  * Performs necessary checks before doing a file copy
  *
@@ -1427,6 +1405,25 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 	loff_t size_in;
 	int ret;
 
+	/*
+	 * Although we now allow filesystems to handle cross sb copy, 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.  NFS defines
+	 * several different file_system_type structures, but they all end up
+	 * using the same ->copy_file_range() function pointer.
+	 */
+	if (file_out->f_op->copy_file_range) {
+		if (file_in->f_op->copy_file_range !=
+		    file_out->f_op->copy_file_range)
+			return -EXDEV;
+	} else if (file_in->f_op->remap_file_range) {
+		if (file_inode(file_in)->i_sb != file_inode(file_out)->i_sb)
+			return -EXDEV;
+	} else {
+                return -EOPNOTSUPP;
+	}
+
 	ret = generic_file_rw_checks(file_in, file_out);
 	if (ret)
 		return ret;
@@ -1495,6 +1492,7 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 
 	file_start_write(file_out);
 
+	ret = -EOPNOTSUPP;
 	/*
 	 * Try cloning first, this is supported by more file systems, and
 	 * more efficient if both clone and copy are supported (e.g. NFS).
@@ -1513,9 +1511,10 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
 		}
 	}
 
-	ret = do_copy_file_range(file_in, pos_in, file_out, pos_out, len,
-				flags);
-	WARN_ON_ONCE(ret == -EOPNOTSUPP);
+	if (file_out->f_op->copy_file_range)
+		ret = file_out->f_op->copy_file_range(file_in, pos_in,
+						      file_out, pos_out,
+						      len, flags);
 done:
 	if (ret > 0) {
 		fsnotify_access(file_in);

  parent 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                                   ` [PATCH v3] vfs: fix copy_file_range regression in cross-fs copies Luis Henriques
2021-02-17 20:47                                     ` 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                             ` Luis Henriques [this message]
2021-02-22 10:46                               ` [PATCH v8] " 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=20210222102456.6692-1-lhenriques@suse.de \
    --to=lhenriques@suse.de \
    --cc=adilger@dilger.ca \
    --cc=aglo@umich.edu \
    --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=hch@infradead.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