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:".
>
> > 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.
I found the reason why I added the CONFIG_FS_VERITY_BUILTIN_SIGNATURES
here: it is related to
https://lore.kernel.org/all/20230705212743.42180-3-ebiggers@xxxxxxxxxx/
where the directory is registered with an element only if
CONFIG_FS_VERITY_BUILTIN_SIGNATURES is defined. I had forgotten, but I
even asked for a clarification on the patch :).
I see that that patch made it to v6.6. So the solution is not to remove
the CONFIG_FS_VERITY_BUILTIN_SIGNATURES, but for me to rebase on top of
a more up to date base.
@Eric: Please get back to me if the patch in
https://lore.kernel.org/all/20230705212743.42180-3-ebiggers@xxxxxxxxxx/
is no longer relevant.
Best.
>
> 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
--
Joel Granados
Attachment:
signature.asc
Description: PGP signature
