linux-kernel - Re: [PATCH 1/6] fs: Add flag to file_system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Feb 2021 09:54:23 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Nicolas Boichat <drinkcat@...omium.org>
Cc:     "Darrick J . Wong" <djwong@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Ian Lance Taylor <iant@...gle.com>,
        Luis Lozano <llozano@...omium.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Dave Chinner <david@...morbit.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/6] fs: Add flag to file_system_type to indicate content
 is generated

On Fri, Feb 12, 2021 at 6:47 AM Nicolas Boichat <drinkcat@...omium.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@...omium.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.