On Wed, Jul 05, 2023 at 02:27:43PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> Currently the registration of the fsverity sysctls happens in
> signature.c, which couples it to CONFIG_FS_VERITY_BUILTIN_SIGNATURES.
>
> This makes it hard to add new sysctls unrelated to builtin signatures.
>
> Also, some users have started checking whether the directory
> /proc/sys/fs/verity exists as a way to tell whether fsverity is
> supported. This isn't the intended method; instead, the existence of
> /sys/fs/$fstype/features/verity should be checked, or users should just
> try to use the fsverity ioctls. Regardlesss, it should be made to work
> as expected without a dependency on CONFIG_FS_VERITY_BUILTIN_SIGNATURES.
>
> Therefore, move the sysctl registration into init.c. With
> CONFIG_FS_VERITY_BUILTIN_SIGNATURES, nothing changes. Without it, but
> with CONFIG_FS_VERITY, an empty list of sysctls is now registered.
>
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
> fs/verity/fsverity_private.h | 1 +
> fs/verity/init.c | 32 ++++++++++++++++++++++++++++++++
> fs/verity/signature.c | 33 +--------------------------------
> 3 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index c5ab9023dd2d3..d071a6e32581e 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -123,6 +123,7 @@ void __init fsverity_init_info_cache(void);
> /* signature.c */
>
> #ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> +extern int fsverity_require_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 bcd11d63eb1ca..a29f062f6047b 100644
> --- a/fs/verity/init.c
> +++ b/fs/verity/init.c
> @@ -9,6 +9,37 @@
>
> #include <linux/ratelimit.h>
>
> +#ifdef CONFIG_SYSCTL
> +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,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = SYSCTL_ZERO,
> + .extra2 = SYSCTL_ONE,
> + },
> +#endif
> + { }
> +};
> +
Just to double check: When CONFIG_FS_VERITY_BUILTIN_SIGNATURES is not
defined then you expect an empty directory under "fs/verity". right?
I'm double checking as part of the ongoing "removing the sentinel" patch
set. Latest patchset here
https://lore.kernel.org/all/20230906-jag-sysctl_remove_empty_elem_arch-v1-0-3935d4854248@xxxxxxxxxxx/.
The filesystem chunk has not yet been publish, but I'm making it ready
for when that time comes.
> +static void __init fsverity_init_sysctl(void)
> +{
> + fsverity_sysctl_header = register_sysctl("fs/verity",
> + fsverity_sysctl_table);
> + if (!fsverity_sysctl_header)
> + panic("fsverity sysctl registration failed");
> +}
> +#else /* CONFIG_SYSCTL */
> +static inline void fsverity_init_sysctl(void)
> +{
> +}
> +#endif /* !CONFIG_SYSCTL */
> +
> void fsverity_msg(const struct inode *inode, const char *level,
> const char *fmt, ...)
> {
> @@ -36,6 +67,7 @@ static int __init fsverity_init(void)
> fsverity_check_hash_algs();
...
Best
--
Joel Granados
Attachment:
signature.asc
Description: PGP signature
