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

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

[Breno Leitao]

When kmemleak is enabled, it incorrectly reports the sfi structure
allocated during efivarfs_init_fs_context() as leaked:

    unreferenced object 0xffff888146250b80 (size 64):
    __kmalloc_cache_noprof
    efivarfs_init_fs_context
    ...

On module unload, this object is freed in efivarfs_kill_sb(), confirming
no actual leak. Also, kfree(sfi) is called at efivarfs_kill_sb(). I am
not able to explain why kmemleak detected it as a leak. To silence this
false-positive, mark the sfi allocation as ignored by kmemleak right
after allocation.

This ensures clearer leak diagnostics for this allocation path.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 fs/efivarfs/super.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index c900d98bf4945..5f867ad2005ae 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -19,6 +19,7 @@
 #include <linux/notifier.h>
 #include <linux/printk.h>
 #include <linux/namei.h>
+#include <linux/kmemleak.h>
 
 #include "internal.h"
 #include "../internal.h"
@@ -498,6 +499,7 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
 	if (!sfi)
 		return -ENOMEM;
 
+	kmemleak_ignore(sfi);
 	sfi->mount_opts.uid = GLOBAL_ROOT_UID;
 	sfi->mount_opts.gid = GLOBAL_ROOT_GID;
 

---
base-commit: 8c2e52ebbe885c7eeaabd3b7ddcdc1246fc400d2
change-id: 20250714-kmemleak_efi-bedfe48a304d

Best regards,
--  
Breno Leitao <leitao@debian.org>

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

[Ard Biesheuvel]

On Tue, 15 Jul 2025 at 19:31, Breno Leitao <leitao@debian.org> wrote:
>
> When kmemleak is enabled, it incorrectly reports the sfi structure
> allocated during efivarfs_init_fs_context() as leaked:
>
>     unreferenced object 0xffff888146250b80 (size 64):
>     __kmalloc_cache_noprof
>     efivarfs_init_fs_context
>     ...
>
> On module unload, this object is freed in efivarfs_kill_sb(), confirming
> no actual leak. Also, kfree(sfi) is called at efivarfs_kill_sb(). I am
> not able to explain why kmemleak detected it as a leak. To silence this
> false-positive, mark the sfi allocation as ignored by kmemleak right
> after allocation.
>
> This ensures clearer leak diagnostics for this allocation path.
>

Can you provide a reproducer? x86 defconfig with kmemleak enabled does
not show this behavior.

In any case, just silencing the diagnostic is not a great way to deal
with this imho.


> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  fs/efivarfs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
> index c900d98bf4945..5f867ad2005ae 100644
> --- a/fs/efivarfs/super.c
> +++ b/fs/efivarfs/super.c
> @@ -19,6 +19,7 @@
>  #include <linux/notifier.h>
>  #include <linux/printk.h>
>  #include <linux/namei.h>
> +#include <linux/kmemleak.h>
>
>  #include "internal.h"
>  #include "../internal.h"
> @@ -498,6 +499,7 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
>         if (!sfi)
>                 return -ENOMEM;
>
> +       kmemleak_ignore(sfi);
>         sfi->mount_opts.uid = GLOBAL_ROOT_UID;
>         sfi->mount_opts.gid = GLOBAL_ROOT_GID;
>
>
> ---
> base-commit: 8c2e52ebbe885c7eeaabd3b7ddcdc1246fc400d2
> change-id: 20250714-kmemleak_efi-bedfe48a304d
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
>

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

[Breno Leitao]

Hello Ard,

On Wed, Jul 16, 2025 at 10:41:24AM +1000, Ard Biesheuvel wrote:
> On Tue, 15 Jul 2025 at 19:31, Breno Leitao <leitao@debian.org> wrote:
> >
> > When kmemleak is enabled, it incorrectly reports the sfi structure
> > allocated during efivarfs_init_fs_context() as leaked:
> >
> >     unreferenced object 0xffff888146250b80 (size 64):
> >     __kmalloc_cache_noprof
> >     efivarfs_init_fs_context
> >     ...
> >
> > On module unload, this object is freed in efivarfs_kill_sb(), confirming
> > no actual leak. Also, kfree(sfi) is called at efivarfs_kill_sb(). I am
> > not able to explain why kmemleak detected it as a leak. To silence this
> > false-positive, mark the sfi allocation as ignored by kmemleak right
> > after allocation.
> >
> > This ensures clearer leak diagnostics for this allocation path.
> >
> 
> Can you provide a reproducer? x86 defconfig with kmemleak enabled does
> not show this behavior.

I see this problem all the time when mounting efivars. This is the
config I am using: https://pastebin.com/i21Yv0jt

Looking further at this problem, I am running this patch to get more
information:

	diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
	index c900d98bf4945..b93ddd5b5cc0d 100644
	--- a/fs/efivarfs/super.c
	+++ b/fs/efivarfs/super.c
	@@ -497,10 +497,13 @@ static int efivarfs_init_fs_context(struct fs_context *fc)
		sfi = kzalloc(sizeof(*sfi), GFP_KERNEL);
		if (!sfi)
			return -ENOMEM;
	+       printk("sfi: Allocated sfi  at %p\n", sfi);
	+       dump_stack();

		sfi->mount_opts.uid = GLOBAL_ROOT_UID;
		sfi->mount_opts.gid = GLOBAL_ROOT_GID;

	+       printk("sfi: previous address %p and new %p\n", fc->s_fs_info, sfi);
		fc->s_fs_info = sfi;
		fc->ops = &efivarfs_context_ops;

	@@ -514,6 +517,8 @@ static void efivarfs_kill_sb(struct super_block *sb)
		blocking_notifier_chain_unregister(&efivar_ops_nh, &sfi->nb);
		kill_litter_super(sb);

	+       printk("sfi: Freeing sfi at %p\n", sfi);
	+       dump_stack();
		kfree(sfi);
	}

The logs are interesting:

Machine has just booted and:

	# mount | grep efivar
	efivarfs on /sys/firmware/efi/efivars type efivarfs (ro,nosuid,nodev,noexec,noatime)

	# dmesg | grep sfi
	[  147.705760] sfi: Allocated sfi  at 00000000fab3df14
	[  148.012125] sfi: previous address 0000000000000000 and new 00000000fab3df14
	[  196.942547] sfi: Allocated sfi  at 00000000f7561519
	[  196.952762] sfi: previous address 0000000000000000 and new 00000000f7561519

	# cat /sys/kernel/debug/kmemleak
	unreferenced object 0xffff88810a1de380 (size 64):
	comm "mount", pid 818, jiffies 4294850435
	hex dump (first 32 bytes):
	00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	backtrace (crc 0):
	__kmalloc_cache_noprof+0x407/0x4f0
	efivarfs_init_fs_context+0x4e/0x1a0 [efivarfs]
	alloc_fs_context+0x4b6/0x860
	path_mount+0x80d/0x1c00
	__x64_sys_mount+0x202/0x270
	do_syscall_64+0x6e/0x390
	entry_SYSCALL_64_after_hwframe+0x4b/0x53

	# sudo umount /sys/firmware/efi/efivars
	# dmesg | grep -i sfi
	[  147.705760] sfi: Allocated sfi  at 00000000fab3df14
	[  148.012125] sfi: previous address 0000000000000000 and new 00000000fab3df14
	[  196.942547] sfi: Allocated sfi  at 00000000f7561519
	[  196.952762] sfi: previous address 0000000000000000 and new 00000000f7561519
	[  366.097658] sfi: Freeing sfi at 00000000fab3df14

	# sudo rmmod efivarfs
	# dmesg | grep -i sfi
	[  147.705760] sfi: Allocated sfi  at 00000000fab3df14
	[  148.012125] sfi: previous address 0000000000000000 and new 00000000fab3df14
	[  196.942547] sfi: Allocated sfi  at 00000000f7561519
	[  196.952762] sfi: previous address 0000000000000000 and new 00000000f7561519
	[  366.097658] sfi: Freeing sfi at 00000000fab3df14

	# cat /sys/kernel/debug/kmemleak
	unreferenced object 0xffff88810a1de380 (size 64):
	comm "mount", pid 818, jiffies 4294850435
	hex dump (first 32 bytes):
	00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
	backtrace (crc 0):
	__kmalloc_cache_noprof+0x407/0x4f0
	0xffffffffa0411d9e
	alloc_fs_context+0x4b6/0x860
	path_mount+0x80d/0x1c00
	__x64_sys_mount+0x202/0x270
	do_syscall_64+0x6e/0x390
	entry_SYSCALL_64_after_hwframe+0x4b/0x53


So, are we somehow leaking memory?!

Here is the stacks, in case you want to see them:

	# dmesg | grep -i sfi -A 30
	[  147.705760] sfi: Allocated sfi  at 00000000fab3df14
	[  147.715844] Tainted: [S]=CPU_OUT_OF_SPEC, [E]=UNSIGNED_MODULE
	[  147.715847] Hardware name: Quanta Twin Lakes MP/Twin Lakes Passive MP, BIOS F09_3A23 12/08/2020
	[  147.715850] Call Trace:
	[  147.715854]  <TASK>
	[  147.715858]  dump_stack_lvl+0xa8/0xc0
	[  147.715872]  efivarfs_init_fs_context+0x6e/0x1a0 [efivarfs]
	[  147.715887]  alloc_fs_context+0x4b6/0x860
	[  147.715904]  path_mount+0x920/0x1c00
	[  147.715922]  ? finish_automount+0x5b0/0x5b0
	[  147.715930]  ? kmem_cache_free+0x318/0x560
	[  147.715938]  ? user_path_at+0x4f/0x60
	[  147.715962]  __x64_sys_mount+0x202/0x270
	[  147.715973]  ? path_mount+0x1c00/0x1c00
	[  147.715997]  do_syscall_64+0x6e/0x390
	[  147.716009]  entry_SYSCALL_64_after_hwframe+0x4b/0x53
	[  147.716015] RIP: 0033:0x7f480bbe794e
	[  147.716022] Code: 48 8b 0d cd 94 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 9a 94 0e 00 f7 d8 64 89 01 48
	[  147.716027] RSP: 002b:00007ffd89a74a28 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
	[  147.716034] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f480bbe794e
	[  147.716038] RDX: 00007f480c0903a7 RSI: 00007ffd89a74a30 RDI: 00007f480c0903a7
	[  147.716042] RBP: 0000000000000004 R08: 0000000000000000 R09: 00007ffd89a748b0
	[  147.716045] R10: 000000000000000e R11: 0000000000000246 R12: 0000000000000000
	[  147.716048] R13: 00007f480c0903a7 R14: 000000000000000e R15: 00007ffd89a74a30
	[  147.716076]  </TASK>
	[  148.012125] sfi: previous address 0000000000000000 and new 00000000fab3df14
	--
	[  196.942547] sfi: Allocated sfi  at 00000000f7561519
	[  196.952507] CPU: 34 UID: 0 PID: 818 Comm: mount Tainted: G S          E       6.16.0-rc6upstream-00004-g9ed31e914181-dirty #77 PREEMPT(none)
	[  196.952520] Tainted: [S]=CPU_OUT_OF_SPEC, [E]=UNSIGNED_MODULE
	[  196.952523] Hardware name: Quanta Twin Lakes MP/Twin Lakes Passive MP, BIOS F09_3A23 12/08/2020
	[  196.952526] Call Trace:
	[  196.952530]  <TASK>
	[  196.952534]  dump_stack_lvl+0xa8/0xc0
	[  196.952547]  efivarfs_init_fs_context+0x6e/0x1a0 [efivarfs]
	[  196.952563]  alloc_fs_context+0x4b6/0x860
	[  196.952581]  path_mount+0x80d/0x1c00
	[  196.952598]  ? finish_automount+0x5b0/0x5b0
	[  196.952606]  ? kmem_cache_free+0x318/0x560
	[  196.952615]  ? user_path_at+0x4f/0x60
	[  196.952638]  __x64_sys_mount+0x202/0x270
	[  196.952650]  ? path_mount+0x1c00/0x1c00
	[  196.952659]  ? getname_flags.part.0+0xfd/0x490
	[  196.952678]  do_syscall_64+0x6e/0x390
	[  196.952690]  entry_SYSCALL_64_after_hwframe+0x4b/0x53
	[  196.952696] RIP: 0033:0x7fdabdd0f94e
	[  196.952705] Code: 48 8b 0d cd 94 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 9a 94 0e 00 f7 d8 64 89 01 48
	[  196.952710] RSP: 002b:00007fff7b4e25a8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
	[  196.952717] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fdabdd0f94e
	[  196.952721] RDX: 0000563cef4deef0 RSI: 0000563cef4deec0 RDI: 0000563cef4e0ef0
	[  196.952725] RBP: 0000563cef4dea00 R08: 0000000000000000 R09: 0000000000000000
	[  196.952728] R10: 000000000000042f R11: 0000000000000246 R12: 0000000000000000
	[  196.952731] R13: 0000563cef4deef0 R14: 0000563cef4e0ef0 R15: 0000563cef4dea00
	[  196.952759]  </TASK>
	[  196.952762] sfi: previous address 0000000000000000 and new 00000000f7561519
	--
	[  366.097658] sfi: Freeing sfi at 00000000fab3df14
	[  366.106994] CPU: 25 UID: 0 PID: 11734 Comm: umount Kdump: loaded Tainted: G S          E       6.16.0-rc6upstream-00004-g9ed31e914181-dirty #77 PREEMPT(none)
	[  366.107005] Tainted: [S]=CPU_OUT_OF_SPEC, [E]=UNSIGNED_MODULE
	[  366.107008] Hardware name: Quanta Twin Lakes MP/Twin Lakes Passive MP, BIOS F09_3A23 12/08/2020
	[  366.107011] Call Trace:
	[  366.107014]  <TASK>
	[  366.107018]  dump_stack_lvl+0xa8/0xc0
	[  366.107031]  efivarfs_kill_sb+0x5d/0x70 [efivarfs]
	[  366.107046]  deactivate_locked_super+0xa2/0x160
	[  366.107058]  cleanup_mnt+0x282/0x3c0
	[  366.107066]  ? trace_irq_enable.constprop.0+0x146/0x1b0
	[  366.107076]  task_work_run+0x12a/0x210
	[  366.107087]  ? task_work_cancel+0x20/0x20
	[  366.107093]  ? __x64_sys_umount+0xfe/0x120
	[  366.107099]  ? rcu_is_watching+0xe/0xb0
	[  366.107110]  exit_to_user_mode_loop+0xff/0x110
	[  366.107117]  do_syscall_64+0x2f0/0x390
	[  366.107125]  entry_SYSCALL_64_after_hwframe+0x4b/0x53
	[  366.107130] RIP: 0033:0x7fd66a10e87b
	[  366.107143] Code: a3 a5 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 90 f3 0f 1e fa 31 f6 e9 05 00 00 00 0f 1f 44 00 00 f3 0f 1e fa b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 69 a5 0e 00 f7 d8
	[  366.107147] RSP: 002b:00007ffec5df41b8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
	[  366.107152] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 00007fd66a10e87b
	[  366.107155] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000055b1db334d40
	[  366.107157] RBP: 000055b1db334b10 R08: 0000000000000000 R09: 00007ffec5df2f40
	[  366.107159] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
	[  366.107161] R13: 000055b1db334d40 R14: 000055b1db334c20 R15: 000055b1db334b10
	[  366.107175]  </TASK>

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

[James Bottomley]

On Wed, 2025-07-16 at 04:45 -0700, Breno Leitao wrote:
> On Wed, Jul 16, 2025 at 10:41:24AM +1000, Ard Biesheuvel wrote:
> > On Tue, 15 Jul 2025 at 19:31, Breno Leitao <leitao@debian.org>
> > wrote:
> > > 
> > > When kmemleak is enabled, it incorrectly reports the sfi
> > > structure allocated during efivarfs_init_fs_context() as leaked:
> > > 
> > >     unreferenced object 0xffff888146250b80 (size 64):
> > >     __kmalloc_cache_noprof
> > >     efivarfs_init_fs_context
> > >     ...
> > > 
> > > On module unload, this object is freed in efivarfs_kill_sb(),
> > > confirming no actual leak. Also, kfree(sfi) is called at
> > > efivarfs_kill_sb(). I am not able to explain why kmemleak
> > > detected it as a leak. To silence this false-positive, mark the
> > > sfi allocation as ignored by kmemleak right after allocation.
> > > 
> > > This ensures clearer leak diagnostics for this allocation path.
> > > 
> > 
> > Can you provide a reproducer? x86 defconfig with kmemleak enabled
> > does not show this behavior.
> 
> I see this problem all the time when mounting efivars. This is the
> config I am using: https://pastebin.com/i21Yv0jt

Actually, there is a way this could be happening: to process the
options, we have to allocate sfi early when the fs_context is
initialized, but it is actually a property of the superblock and is
freed when the superblock is killed.  If we got a situation where
something did a final put of the fs context before we get to fill_super
(which is where the handover happens) we would leak sfi's.  This would
have to be on an error leg I presume (or possibly a reconfigure looking
at the code).

If the theory is correct, the leak is genuine and we need to implement
.free in efivarfs_context_ops to fix it.

Regards,

James

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

[James Bottomley]

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,

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

[Breno Leitao]

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@debian.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@HansenPartnership.com>
    Signed-off-by: Breno Leitao <leitao@debian.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,

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

[James Bottomley]

On Wed, 2025-07-16 at 06:16 -0700, Breno Leitao wrote:
> 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@debian.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@HansenPartnership.com>
>     Signed-off-by: Breno Leitao <leitao@debian.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;

Here, you'll excite the coccinelle checkers looking for if(x) free(x)
because free() already also has a test for NULL.

Other than that elision, it looks fine to me.

Regards,

James

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

[Breno Leitao]

On Wed, Jul 16, 2025 at 09:26:03AM -0400, James Bottomley wrote:
> On Wed, 2025-07-16 at 06:16 -0700, Breno Leitao wrote:

> > 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;
> 
> Here, you'll excite the coccinelle checkers looking for if(x) free(x)
> because free() already also has a test for NULL.

Good point.

> Other than that elision, it looks fine to me.

Thanks. I will send a v2.
--breno