Received: by 2002:a05:6a10:8c0a:0:0:0:0 with SMTP id go10csp7400390pxb; Thu, 18 Feb 2021 09:06:28 -0800 (PST) X-Google-Smtp-Source: ABdhPJyjOH9IsDRR4Xby7hTFl2CCnIKzSQ3/ofl7xzJprbFcQQtqoEnvvp69dg/TvxrszjlLBlk+ X-Received: by 2002:a17:906:2697:: with SMTP id t23mr4913132ejc.357.1613667988410; Thu, 18 Feb 2021 09:06:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1613667988; cv=none; d=google.com; s=arc-20160816; b=LxxZ7X/Kz1a99TeYHNuhutfCxhPC5ASwEv9krYUPntUuUOU1bGUTLBOIVuwJgA3b09 hyzPoAlKSDuLvfj5tYmi7XcQfpqfPPJw77r+VoirYpGfbddXptNUBXXp7n+cEX7cUDnR 1MIkJJ02dLxT33GTW4WU4TqCdfp0l7JEShtiJuqyAIP+MHSlVAFhGtCX8Z3ojn9bQT3A V9FzTwJRQQo9vbrdR9gxQoxHTMMUnB+mC6sXXDTXUVB9DPdNWPfJgyDhKY6FnYn+8WM3 2VImWC/CqKbOFadUA1C2zcCtE6M5XJROzWEcAX7jSG+ZASTSfrcpxqdGtRDOsm2MdXDs WIRA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=kVKDeldryA0WRlNtDrfzlt+o1Lgk6qYZZNHg58zxh0Q=; b=wyD9uXj+KHQL0BTNjj2eCyzrTeyjqrCvDS8wQYDvtjuHiCalzwp+GpGASvg9fF+Wlz s60ugq5ZtJkumPvnmiUB1P96rX5Aze41I1zEaq0o11vLBr/rn+wKrveiWBpidwVI6hve eQGeuymvLu2E/2dLcG/y3XZ8M9Uh6cQXtp4JOSSYX3mlp005NoZ9bdrKuBnFRh0jWcf8 3YkMVoohy4LbnHYFbPOlreO3IrguOy7LU9d1x6uTrLQ+8BTEe66J2eQ81i0WgwX8XiIH S+bv+TP6CstVnCEisxcMzCzRA1YPgIdTg2aPs3jyEYJFZNWyTbCCof+uLlOI8tpoeDqb gC3A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id i8si3786686edy.413.2021.02.18.09.06.03; Thu, 18 Feb 2021 09:06:28 -0800 (PST) Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230326AbhBRRDD (ORCPT + 99 others); Thu, 18 Feb 2021 12:03:03 -0500 Received: from mx2.suse.de ([195.135.220.15]:41296 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232474AbhBROgT (ORCPT ); Thu, 18 Feb 2021 09:36:19 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id E7E4FADDB; Thu, 18 Feb 2021 14:35:33 +0000 (UTC) Received: from localhost (brahms [local]) by brahms (OpenSMTPD) with ESMTPA id 2ad944f8; Thu, 18 Feb 2021 14:36:36 +0000 (UTC) From: Luis Henriques To: Amir Goldstein , Jeff Layton , Steve French , Miklos Szeredi , Trond Myklebust , Anna Schumaker , Alexander Viro , "Darrick J. Wong" , Dave Chinner , Greg KH , Nicolas Boichat , Ian Lance Taylor , Luis Lozano , Andreas Dilger , Olga Kornievskaia , Christoph Hellwig 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 Subject: [PATCH v4] vfs: fix copy_file_range regression in cross-fs copies Date: Thu, 18 Feb 2021 14:36:35 +0000 Message-Id: <20210218143635.24916-1-lhenriques@suse.de> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org 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 use generic_copy_file_range() instead of vfs_copy_file_range() so that it can still fall-back to splice without going through all the checks. 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 Signed-off-by: Luis Henriques --- And here's v4. I'd like to request help for testing. I know Nicolas is doing that (thanks! and thanks for the reviews). But it would be great to get at least the nfs code tested. Olga, can you help here? 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 | 2 +- fs/read_write.c | 50 +++++++++++++++++++++++-------------------------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 04937e51de56..49dd28ee2602 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -578,7 +578,7 @@ 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 generic_copy_file_range(src, src_pos, dst, dst_pos, count, 0); } __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..214d44f7cbfa 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; @@ -1499,8 +1496,7 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, * Try cloning first, this is supported by more file systems, and * more efficient if both clone and copy are supported (e.g. NFS). */ - if (file_in->f_op->remap_file_range && - file_inode(file_in)->i_sb == file_inode(file_out)->i_sb) { + if (file_in->f_op->remap_file_range) { loff_t cloned; cloned = file_in->f_op->remap_file_range(file_in, pos_in, @@ -1513,9 +1509,9 @@ 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); + 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);