From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6C05CC433E0 for ; Fri, 12 Feb 2021 04:59:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 386DC64E76 for ; Fri, 12 Feb 2021 04:59:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229794AbhBLE7v (ORCPT ); Thu, 11 Feb 2021 23:59:51 -0500 Received: from mail.kernel.org ([198.145.29.99]:39504 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229575AbhBLE7o (ORCPT ); Thu, 11 Feb 2021 23:59:44 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 7B2AC64E70; Fri, 12 Feb 2021 04:59:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1613105943; bh=izbQjwna7XZsPlb3xdcIvo7CctQA3iJf8QX043cAiT4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=b+/6gGzv0s/IzrB6GvOareNQ4+chnYH0WbwR/TidFZK5LAfN8wnWOCp/FtMWAq73X INn2OZuYkdvcbb/LCaOZT3yR/F5S4s2At48KACJNpA0ZtQlXmJgyZulKOs883UucOK +XF7aUrUYpHNnTx7G+HDO/fCh0H5zuQ+wZcW1UzIZsxtYSP4uBEKO+TFEUUQmHZMFw D8mtHDEOjaqWr97Du4KxLcTOcPOj1B9fbZHyOayP3Dnr7TEQHR9pvYYXoDr92sH6U6 +7NKnjoz/FxvQO4hQxqkydEoyXUBBLviWvY5EkwFyvoMAMuB/PEFSiC+rSGvaOCKgq c089WPu6YFWRw== Date: Thu, 11 Feb 2021 20:59:02 -0800 From: "Darrick J. Wong" To: Nicolas Boichat Cc: Alexander Viro , Ian Lance Taylor , Luis Lozano , Greg KH , Dave Chinner , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 6/6] vfs: Disallow copy_file_range on generated file systems Message-ID: <20210212045902.GG7187@magnolia> References: <20210212044405.4120619-1-drinkcat@chromium.org> <20210212124354.6.Idc9c3110d708aa0df9d8fe5a6246524dc8469dae@changeid> <20210212045347.GM7190@magnolia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210212045347.GM7190@magnolia> Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Thu, Feb 11, 2021 at 08:53:47PM -0800, Darrick J. Wong wrote: > On Fri, Feb 12, 2021 at 12:44:05PM +0800, Nicolas Boichat wrote: > > copy_file_range (which calls generic_copy_file_checks) uses the > > inode file size to adjust the copy count parameter. This breaks > > with special filesystems like procfs/sysfs/debugfs/tracefs, where > > the file size appears to be zero, but content is actually returned > > when a read operation is performed. Other issues would also > > happen on partial writes, as the function would attempt to seek > > in the input file. > > > > Use the newly introduced FS_GENERATED_CONTENT filesystem flag > > to return -EOPNOTSUPP: applications can then retry with a more > > usual read/write based file copy (the fallback code is usually > > already present to handle older kernels). > > > > Signed-off-by: Nicolas Boichat > > --- > > > > fs/read_write.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/read_write.c b/fs/read_write.c > > index 0029ff2b0ca8..80322e89fb0a 100644 > > --- a/fs/read_write.c > > +++ b/fs/read_write.c > > @@ -1485,6 +1485,9 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in, > > if (flags != 0) > > return -EINVAL; > > > > + if (file_inode(file_in)->i_sb->s_type->fs_flags & FS_GENERATED_CONTENT) > > + return -EOPNOTSUPP; > > Why not declare a dummy copy_file_range_nop function that returns > EOPNOTSUPP and point all of these filesystems at it? > > (Or, I guess in these days where function pointers are the enemy, > create a #define that is a cast of 0x1, and fix do_copy_file_range to > return EOPNOTSUPP if it sees that?) Oh, I see, because that doesn't help if the source file is procfs and the dest file is (say) xfs, because the generic version will try to do splice magic and *poof*. I guess the other nit thatI can think of at this late hour is ... what about the other virtual filesystems like configfs and whatnot? Should we have a way to flag them as "this can't be the source of a CFR request" as well? Or is it just trace/debug/proc/sysfs that have these "zero size but readable" speshul behaviors? --D > > --D > > > + > > ret = generic_copy_file_checks(file_in, pos_in, file_out, pos_out, &len, > > flags); > > if (unlikely(ret)) > > -- > > 2.30.0.478.g8a0d178c01-goog > >