linux-kernel - Re: [PATCH] efivarfs: Suppress false-positive kmemleak warning for sfi

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: <qdw7skjlcw6dvnewpfrtxc27pm7sroan5eyn53exndehp3blav@z25oqkoo3ohw>
Date: Wed, 16 Jul 2025 06:16:32 -0700
From: Breno Leitao <leitao@...ian.org>
To: James Bottomley <James.Bottomley@...senpartnership.com>
Cc: Ard Biesheuvel <ardb@...nel.org>, Jeremy Kerr <jk@...abs.org>, 
	linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH] efivarfs: Suppress false-positive kmemleak warning for
 sfi

On Wed, Jul 16, 2025 at 09:09:00AM -0400, James Bottomley wrote:
> On Wed, 2025-07-16 at 08:31 -0400, James Bottomley wrote:
> [...]
> > If the theory is correct, the leak is genuine and we need to
> > implement .free in efivarfs_context_ops to fix it.
> 
> Rather than trying to trace this, which will be hard, it might be
> easier just to try the fix below (not even compile tested) and see if
> it works.  Note there's no danger of a double free because when fc-
> >s_fs_info is copied to sb->s_fs_info, the field is nulled in fc.
> 
> Regards,
> 
> James
> 
> ---
> 
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index c900d98bf494..90a619d027fd 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -390,10 +390,16 @@ static int efivarfs_reconfigure(struct fs_context *fc)
>  	return 0;
>  }
>  
> +static void efivarfs_fs_context_free(struct fs_context *fc)
> +{
> +	kfree(fc->s_fs_info);
> +}
> +
>  static const struct fs_context_operations efivarfs_context_ops = {
>  	.get_tree	= efivarfs_get_tree,
>  	.parse_param	= efivarfs_parse_param,
>  	.reconfigure	= efivarfs_reconfigure,
> +	.free		= efivarfs_fs_context_free,
>  };
>  
>  static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor,

Hello James,

I was testing something very similar based on your previous email. I can
confirm that the following patch make kmemleak happy. 

Regarding the fixes, I think this was introduced in commit
5329aa5101f73c ("efivarfs: Add uid/gid mount options")

commit 035521e8a5029ea814337d680e0552ccab1f97e2
Author: Breno Leitao <leitao@...ian.org>
Date:   Wed Jul 16 06:08:57 2025 -0700

    efivarfs: Fix memory leak of efivarfs_fs_info in fs_context error paths
    
    When processing mount options, efivarfs allocates efivarfs_fs_info (sfi)
    early in fs_context initialization. However, sfi is associated with the
    superblock and typically freed when the superblock is destroyed. If the
    fs_context is released (final put) before fill_super is called—such as
    on error paths or during reconfiguration—the sfi structure would leak,
    as ownership never transfers to the superblock.
    
    Implement the .free callback in efivarfs_context_ops to ensure any
    allocated sfi is properly freed if the fs_context is torn down before
    fill_super, preventing this memory leak.
    
    Suggested-by: James Bottomley <James.Bottomley@...senPartnership.com>
    Signed-off-by: Breno Leitao <leitao@...ian.org>

diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index c900d98bf4945..07a3b9293396b 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -390,10 +390,22 @@ static int efivarfs_reconfigure(struct fs_context *fc)
 	return 0;
 }
 
+static void efivarfs_free(struct fs_context *fc)
+{
+	struct efivarfs_fs_info *sfi;
+
+	sfi = fc->s_fs_info;
+	if (!sfi)
+		return;
+
+	kfree(sfi);
+}
+
 static const struct fs_context_operations efivarfs_context_ops = {
 	.get_tree	= efivarfs_get_tree,
 	.parse_param	= efivarfs_parse_param,
 	.reconfigure	= efivarfs_reconfigure,
+	.free		= efivarfs_free,
 };
 
 static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor,