diff mbox series

fsverity: use register_sysctl_init() to avoid kmemleak warning

Message ID 20240501025331.594183-1-ebiggers@kernel.org (mailing list archive)
State Accepted
Headers show
Series fsverity: use register_sysctl_init() to avoid kmemleak warning | expand

Commit Message

Eric Biggers May 1, 2024, 2:53 a.m. UTC
From: Eric Biggers <ebiggers@google.com>

Since the fsverity sysctl registration runs as a builtin initcall, there
is no corresponding sysctl deregistration and the resulting struct
ctl_table_header is not used.  This can cause a kmemleak warning just
after the system boots up.  (A pointer to the ctl_table_header is stored
in the fsverity_sysctl_header static variable, which kmemleak should
detect; however, the compiler can optimize out that variable.)  Avoid
the kmemleak warning by using register_sysctl_init() which is intended
for use by builtin initcalls and uses kmemleak_not_leak().

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Closes: https://lore.kernel.org/r/CAHj4cs8DTSvR698UE040rs_pX1k-WVe7aR6N2OoXXuhXJPDC-w@mail.gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/verity/init.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)


base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354

Comments

Joel Granados May 3, 2024, 2 p.m. UTC | #1
On Tue, Apr 30, 2024 at 07:53:31PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Since the fsverity sysctl registration runs as a builtin initcall, there
> is no corresponding sysctl deregistration and the resulting struct
> ctl_table_header is not used.  This can cause a kmemleak warning just
> after the system boots up.  (A pointer to the ctl_table_header is stored
> in the fsverity_sysctl_header static variable, which kmemleak should
> detect; however, the compiler can optimize out that variable.)  Avoid
> the kmemleak warning by using register_sysctl_init() which is intended
> for use by builtin initcalls and uses kmemleak_not_leak().
> 
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Closes: https://lore.kernel.org/r/CAHj4cs8DTSvR698UE040rs_pX1k-WVe7aR6N2OoXXuhXJPDC-w@mail.gmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/verity/init.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/verity/init.c b/fs/verity/init.c
> index cb2c9aac61ed..f440f0e61e3e 100644
> --- a/fs/verity/init.c
> +++ b/fs/verity/init.c
> @@ -8,12 +8,10 @@
>  #include "fsverity_private.h"
>  
>  #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,
> @@ -26,14 +24,11 @@ static struct ctl_table fsverity_sysctl_table[] = {
>  #endif
>  };
>  
>  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");
> +	register_sysctl_init("fs/verity", fsverity_sysctl_table);
>  }
>  #else /* CONFIG_SYSCTL */
>  static inline void fsverity_init_sysctl(void)
>  {
>  }
> 
> base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354
> -- 
> 2.45.0
> 
LGTM

Reviewed-by: Joel Granados <j.granados@samsung.com>
Eric Biggers May 3, 2024, 3:30 p.m. UTC | #2
On Tue, Apr 30, 2024 at 07:53:31PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Since the fsverity sysctl registration runs as a builtin initcall, there
> is no corresponding sysctl deregistration and the resulting struct
> ctl_table_header is not used.  This can cause a kmemleak warning just
> after the system boots up.  (A pointer to the ctl_table_header is stored
> in the fsverity_sysctl_header static variable, which kmemleak should
> detect; however, the compiler can optimize out that variable.)  Avoid
> the kmemleak warning by using register_sysctl_init() which is intended
> for use by builtin initcalls and uses kmemleak_not_leak().
> 
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Closes: https://lore.kernel.org/r/CAHj4cs8DTSvR698UE040rs_pX1k-WVe7aR6N2OoXXuhXJPDC-w@mail.gmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Applied to https://git.kernel.org/pub/scm/fs/fsverity/linux.git/log/?h=for-next

- Eric
diff mbox series

Patch

diff --git a/fs/verity/init.c b/fs/verity/init.c
index cb2c9aac61ed..f440f0e61e3e 100644
--- a/fs/verity/init.c
+++ b/fs/verity/init.c
@@ -8,12 +8,10 @@ 
 #include "fsverity_private.h"
 
 #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,
@@ -26,14 +24,11 @@  static struct ctl_table fsverity_sysctl_table[] = {
 #endif
 };
 
 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");
+	register_sysctl_init("fs/verity", fsverity_sysctl_table);
 }
 #else /* CONFIG_SYSCTL */
 static inline void fsverity_init_sysctl(void)
 {
 }