linux-kernel - Re: [PATCH 2/4] aio: Remove the now superfluous sentinel elements from ctl

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:   Tue, 7 Nov 2023 19:42:31 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     j.granados@...sung.com
Cc:     Luis Chamberlain <mcgrof@...nel.org>, willy@...radead.org,
        josh@...htriplett.org, Kees Cook <keescook@...omium.org>,
        David Howells <dhowells@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Benjamin LaHaise <bcrl@...ck.org>,
        Eric Biederman <ebiederm@...ssion.com>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna@...nel.org>,
        Chuck Lever <chuck.lever@...cle.com>,
        Jeff Layton <jlayton@...nel.org>, Neil Brown <neilb@...e.de>,
        Olga Kornievskaia <kolga@...app.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>,
        Jan Kara <jack@...e.cz>, Amir Goldstein <amir73il@...il.com>,
        Matthew Bobrowski <repnop@...gle.com>,
        Anton Altaparmakov <anton@...era.com>,
        Namjae Jeon <linkinjeon@...nel.org>,
        Mark Fasheh <mark@...heh.com>,
        Joel Becker <jlbec@...lplan.org>,
        Joseph Qi <joseph.qi@...ux.alibaba.com>,
        Iurii Zaikin <yzaikin@...gle.com>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Chandan Babu R <chandan.babu@...cle.com>,
        "Darrick J. Wong" <djwong@...nel.org>,
        Jan Harkes <jaharkes@...cmu.edu>, coda@...cmu.edu,
        linux-cachefs@...hat.com, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-aio@...ck.org,
        linux-mm@...ck.org, linux-nfs@...r.kernel.org,
        linux-ntfs-dev@...ts.sourceforge.net, ocfs2-devel@...ts.linux.dev,
        fsverity@...ts.linux.dev, linux-xfs@...r.kernel.org,
        codalist@...a.cs.cmu.edu
Subject: Re: [PATCH 2/4] aio: Remove the now superfluous sentinel elements
 from ctl_table array

On Tue, Nov 07, 2023 at 02:44:21PM +0100, Joel Granados via B4 Relay wrote:
> [PATCH 2/4] aio: Remove the now superfluous sentinel elements from ctl_table array

The commit prefix should be "fs:".

> Remove sentinel elements ctl_table struct. Special attention was placed in
> making sure that an empty directory for fs/verity was created when
> CONFIG_FS_VERITY_BUILTIN_SIGNATURES is not defined. In this case we use the
> register sysctl call that expects a size.
[...]
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index d071a6e32581..8191bf7ad706 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -122,8 +122,8 @@ void __init fsverity_init_info_cache(void);
>  
>  /* signature.c */
>  
> -#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
>  extern int fsverity_require_signatures;
> +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
>  int fsverity_verify_signature(const struct fsverity_info *vi,
>  			      const u8 *signature, size_t sig_size);
>  
> diff --git a/fs/verity/init.c b/fs/verity/init.c
> index a29f062f6047..e31045dd4f6c 100644
> --- a/fs/verity/init.c
> +++ b/fs/verity/init.c
> @@ -13,7 +13,6 @@
>  static struct ctl_table_header *fsverity_sysctl_header;
>  
>  static struct ctl_table fsverity_sysctl_table[] = {
> -#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
>  	{
>  		.procname       = "require_signatures",
>  		.data           = &fsverity_require_signatures,
> @@ -23,14 +22,17 @@ static struct ctl_table fsverity_sysctl_table[] = {
>  		.extra1         = SYSCTL_ZERO,
>  		.extra2         = SYSCTL_ONE,
>  	},
> -#endif
> -	{ }
>  };
>  
>  static void __init fsverity_init_sysctl(void)
>  {
> +#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
>  	fsverity_sysctl_header = register_sysctl("fs/verity",
>  						 fsverity_sysctl_table);
> +#else
> +	fsverity_sysctl_header = register_sysctl_sz("fs/verity",
> +						 fsverity_sysctl_table, 0);
> +#endif
>  	if (!fsverity_sysctl_header)
>  		panic("fsverity sysctl registration failed");

This does not make sense, and it causes a build error when CONFIG_FS_VERITY=y
and CONFIG_FS_VERITY_BUILTIN_SIGNATURES=n.

I think all you need to do is delete the sentinel element, the same as
everywhere else.  I just tested it, and it works fine.

BTW, the comments for register_sysctl_sz() and __register_sysctl_table() are
outdated, as they still say "A completely 0 filled entry terminates the table."

- Eric