Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210212044405.4120619-1-drinkcat@chromium.org> <20210212124354.1.I7084a6235fbcc522b674a6b1db64e4aff8170485@changeid>
In-Reply-To: <20210212124354.1.I7084a6235fbcc522b674a6b1db64e4aff8170485@changeid>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Fri, 12 Feb 2021 09:54:23 +0200
Message-ID: <CAOQ4uxgZzbDV3REOBbq42u9VR8byLpwn7zBuyrePxY+1cJwM3w@mail.gmail.com>
Subject: Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content
 is generated
To:     Nicolas Boichat <drinkcat@chromium.org>
Cc:     "Darrick J . Wong" <djwong@kernel.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Ian Lance Taylor <iant@google.com>,
        Luis Lozano <llozano@chromium.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Dave Chinner <david@fromorbit.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Feb 12, 2021 at 6:47 AM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> Filesystems such as procfs and sysfs generate their content at
> runtime. This implies the file sizes do not usually match the
> amount of data that can be read from the file, and that seeking
> may not work as intended.
>
> This will be useful to disallow copy_file_range with input files
> from such filesystems.
>
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
> ---
> I first thought of adding a new field to struct file_operations,
> but that doesn't quite scale as every single file creation
> operation would need to be modified.
>
>  include/linux/fs.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3482146b11b0..5bd58b928e94 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2335,6 +2335,7 @@ struct file_system_type {
>  #define FS_ALLOW_IDMAP         32      /* FS has been updated to handle vfs idmappings. */
>  #define FS_THP_SUPPORT         8192    /* Remove once all fs converted */
>  #define FS_RENAME_DOES_D_MOVE  32768   /* FS will handle d_move() during rename() internally. */
> +#define FS_GENERATED_CONTENT   65536   /* FS contains generated content */

Can you please make the flag name a little less arbitrary.

Either something that conveys the facts as they are (e.g. "zero size
but readable")
or anything that you think describes best the special behavior that follows from
observing this flag.

The alternative is for the flag name to express what you want
(e.g. "don't copy file range") like FS_DISALLOW_NOTIFY_PERM.

Also, I wonder. A great deal of the files you target are opened with seq_open()
(I didn't audit all of them). Maybe it's worth setting an FMODE flag
in seq_open()
and some of it's relatives to express the quality of the file instead
of flagging
the filesystem? Maybe we can do both to cover more cases.

Thanks,
Amir.