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]

Message-ID: <20231108082648.nne6k2unowzhhg5m@localhost>
Date:   Wed, 8 Nov 2023 09:26:48 +0100
From:   Joel Granados <j.granados@...sung.com>
To:     Eric Biggers <ebiggers@...nel.org>
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@...emann.coda.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 07:42:31PM -0800, Eric Biggers wrote:
> 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:".
Will do

> 
> > 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.
Indeed. good catch and thx for testing. I'll adjust it on my V2.

> 
> 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."
For now this is still "technically" correct. However, this will be
removed on the last patchset when we actually delete the check for the
sentinel element.
For now I'll leave it like it is, but I'll double check to make sure
that I remove it at the end.

Best
> 
> - Eric

-- 

Joel Granados

Download attachment "signature.asc" of type "application/pgp-signature" (660 bytes)