[PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Mehdi Ben Hadj Khelifa]

Failure in setup_bdev_super() triggers an error path where
fc->s_fs_info ownership has already been transferred to the superblock via
sget_fc() call in get_tree_bdev_flags() and calling put_fs_context() in
do_new_mount() to free the s_fs_info for the specific filesystem gets
passed in a NULL pointer.

Pass back the ownership of the s_fs_info pointer to the filesystem context
once the error path has been triggered to be cleaned up gracefully in
put_fs_context().

Fixes: cb50b348c71f ("convenience helpers: vfs_get_super() and sget_fc()")
Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
Note:This patch might need some more testing as I only did run selftests 
with no regression, check dmesg output for no regression, run reproducer 
with no bug.

fs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 5bab94fb7e03..8fadf97fcc42 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1690,6 +1690,11 @@ int get_tree_bdev_flags(struct fs_context *fc,
 		if (!error)
 			error = fill_super(s, fc);
 		if (error) {
+			/*
+			 * return s_fs_info ownership to fc to be cleaned up by put_fs_context()
+			 */
+			fc->s_fs_info = s->s_fs_info;
+			s->s_fs_info = NULL;
 			deactivate_locked_super(s);
 			return error;
 		}
-- 
2.51.2

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Al Viro]

On Fri, Nov 14, 2025 at 05:52:27PM +0100, Mehdi Ben Hadj Khelifa wrote:
> Failure in setup_bdev_super() triggers an error path where
> fc->s_fs_info ownership has already been transferred to the superblock via
> sget_fc() call in get_tree_bdev_flags() and calling put_fs_context() in
> do_new_mount() to free the s_fs_info for the specific filesystem gets
> passed in a NULL pointer.
> 
> Pass back the ownership of the s_fs_info pointer to the filesystem context
> once the error path has been triggered to be cleaned up gracefully in
> put_fs_context().
> 
> Fixes: cb50b348c71f ("convenience helpers: vfs_get_super() and sget_fc()")
> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> ---
> Note:This patch might need some more testing as I only did run selftests 
> with no regression, check dmesg output for no regression, run reproducer 
> with no bug.

Almost certainly bogus; quite a few fill_super() callbacks seriously count
upon "->kill_sb() will take care care of cleanup if we return an error".

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Mehdi Ben Hadj Khelifa]

On 11/18/25 3:59 PM, Al Viro wrote:
> On Fri, Nov 14, 2025 at 05:52:27PM +0100, Mehdi Ben Hadj Khelifa wrote:
>> Failure in setup_bdev_super() triggers an error path where
>> fc->s_fs_info ownership has already been transferred to the superblock via
>> sget_fc() call in get_tree_bdev_flags() and calling put_fs_context() in
>> do_new_mount() to free the s_fs_info for the specific filesystem gets
>> passed in a NULL pointer.
>>
>> Pass back the ownership of the s_fs_info pointer to the filesystem context
>> once the error path has been triggered to be cleaned up gracefully in
>> put_fs_context().
>>
>> Fixes: cb50b348c71f ("convenience helpers: vfs_get_super() and sget_fc()")
>> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>> ---
>> Note:This patch might need some more testing as I only did run selftests
>> with no regression, check dmesg output for no regression, run reproducer
>> with no bug.
> 
> Almost certainly bogus; quite a few fill_super() callbacks seriously count
> upon "->kill_sb() will take care care of cleanup if we return an error".

So should I then free the allocated s_fs_info in the kill_block_super 
instead and check for the null pointer in put_fs_context to not execute 
kfree in subsequent call to hfs_free_fc()?

Because the error generated in setup_bdev_super() when returned to 
do_new_mount() (after a lot of error propagation) it doesn't get handled:	
	if (!err)
		err = do_new_mount_fc(fc, path, mnt_flags);
	put_fs_context(fc);
	return err;

Also doesn't get handled anywhere in the call stack after IIUC:

In path_mount:
	return do_new_mount(path, type_page, sb_flags, mnt_flags, dev_name,
			    data_page);

In do_mount:
	return path_mount(dev_name, &path, type_page, flags, data_page);

So what is recommended in this case ?

Best Regards,
Mehdi Ben Hadj Khelifa

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Al Viro]

On Tue, Nov 18, 2025 at 05:21:59PM +0100, Mehdi Ben Hadj Khelifa wrote:

> > Almost certainly bogus; quite a few fill_super() callbacks seriously count
> > upon "->kill_sb() will take care care of cleanup if we return an error".
> 
> So should I then free the allocated s_fs_info in the kill_block_super
> instead and check for the null pointer in put_fs_context to not execute
> kfree in subsequent call to hfs_free_fc()?

Huh?  How the hell would kill_block_super() know what to do with ->s_fs_info
for that particular fs type?  kill_block_super() is a convenience helper,
no more than that...

> Because the error generated in setup_bdev_super() when returned to
> do_new_mount() (after a lot of error propagation) it doesn't get handled:	
> 	if (!err)
> 		err = do_new_mount_fc(fc, path, mnt_flags);
> 	put_fs_context(fc);
> 	return err;

Would be hard to handle something that is already gone, wouldn't it?
deactivate_locked_super() after the fill_super() failure is where
the superblock is destroyed - nothing past that point could possibly
be of any use.

I would still like the details on the problem you are seeing.

Normal operation (for filesystems that preallocate ->s_fs_info and hang
it off fc) goes like this:

	* fc->s_fs_info is allocated in ->init_fs_context()
	* it is modified (possibly) in ->parse_param()
	* eventually ->get_tree() is called and at some point it
asks for superblock by calling sget_fc().  It may fail (in which
case fc->s_fs_info stays where it is), if may return a preexisting
superblock (ditto) *OR* it may create and return a new superblock.
In that case fc->s_fs_info is no more - it's been moved over to
sb->s_fs_info.  NULL is left behind.  From that point on the
responsibility for that sucker is with the filesystem; nothing in
VFS has any idea where to find it.

Again, there is no such thing as transferring it back to fc - once
fill_super() has been called, there might be any number of additional
things that need to be undone.

For HFS I would expect that hfs_fill_super() would call hfs_mdb_put(sb)
on all failures and have it called from subsequent ->put_super() if
we succeed and later unmount the filesystem.  That seems to be where
->s_fs_info is taken out of superblock and freed.

What do you observe getting leaked and in which case does that happen?

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Al Viro]

On Tue, Nov 18, 2025 at 04:35:09PM +0000, Al Viro wrote:

> For HFS I would expect that hfs_fill_super() would call hfs_mdb_put(sb)
> on all failures and have it called from subsequent ->put_super() if
> we succeed and later unmount the filesystem.  That seems to be where
> ->s_fs_info is taken out of superblock and freed.
> 
> What do you observe getting leaked and in which case does that happen?

AFAICS, the problem is with aca740cecbe5 "fs: open block device after superblock
creation" where you get a failure exit stuck between getting a new superblock
from sget_fc() and calling fill_super().

That is where the gap has been introduced.  I see two possible solutions:
one is to have failure of setup_bdev_super() (and only it) steal ->s_fs_info
back, on the theory that filesystem didn't have a chance to do anything
yet.  Another is to move the call of hfs_mdb_put() from failure exits of
hfs_fill_super() *and* from hfs_put_super() into hfs_kill_sb(), that
would do that:

	generic_shutdown_super(sb);
	hfs_mdb_put(sb);
	if (sb->s_bdev) {
		sync_blockdev(sb->s_bdev);
		bdev_fput(sb->s_bdev_file);
	}

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Mehdi Ben Hadj Khelifa]

On 11/18/25 5:55 PM, Al Viro wrote:
> On Tue, Nov 18, 2025 at 04:35:09PM +0000, Al Viro wrote:
> 


>> For HFS I would expect that hfs_fill_super() would call hfs_mdb_put(sb)
>> on all failures and have it called from subsequent ->put_super() if
>> we succeed and later unmount the filesystem.  That seems to be where
>> ->s_fs_info is taken out of superblock and freed.
>>
>> What do you observe getting leaked and in which case does that happen?
Sorry for my other late reply. My thunderbird client had some issues and 
got delayed and seperated emails somehow...
> 
> AFAICS, the problem is with aca740cecbe5 "fs: open block device after superblock
> creation" where you get a failure exit stuck between getting a new superblock
> from sget_fc() and calling fill_super().
> 

Yes this is what I mentionned in my earlier mail.(not the commit causing 
the issue though).
> That is where the gap has been introduced.  I see two possible solutions:
> one is to have failure of setup_bdev_super() (and only it) steal ->s_fs_info
> back, on the theory that filesystem didn't have a chance to do anything
> yet.  Another is to move the call of hfs_mdb_put() from failure exits of
> hfs_fill_super() *and* from hfs_put_super() into hfs_kill_sb(), that
> would do that:
> 
> 	generic_shutdown_super(sb);
> 	hfs_mdb_put(sb);
> 	if (sb->s_bdev) {
> 		sync_blockdev(sb->s_bdev);
> 		bdev_fput(sb->s_bdev_file);
> 	}

I will do the second approach, test it send it for review shortly.

Best regards,
Mehdi Ben Hadj Khelifa

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[Mehdi Ben Hadj Khelifa]

On 11/18/25 5:35 PM, Al Viro wrote:
> On Tue, Nov 18, 2025 at 05:21:59PM +0100, Mehdi Ben Hadj Khelifa wrote:
> 
>>> Almost certainly bogus; quite a few fill_super() callbacks seriously count
>>> upon "->kill_sb() will take care care of cleanup if we return an error".
>>
>> So should I then free the allocated s_fs_info in the kill_block_super
>> instead and check for the null pointer in put_fs_context to not execute
>> kfree in subsequent call to hfs_free_fc()?
> 
> Huh?  How the hell would kill_block_super() know what to do with ->s_fs_info
> for that particular fs type?  kill_block_super() is a convenience helper,
> no more than that...
> 
Yes, I missed that. Since i only looked at the hfs_free_fc(), I forgot 
that in kill_block_super() it handles all fs types not only hfs which 
only frees s_fs_info.

>> Because the error generated in setup_bdev_super() when returned to
>> do_new_mount() (after a lot of error propagation) it doesn't get handled:	
>> 	if (!err)
>> 		err = do_new_mount_fc(fc, path, mnt_flags);
>> 	put_fs_context(fc);
>> 	return err;
> 
> Would be hard to handle something that is already gone, wouldn't it?
> deactivate_locked_super() after the fill_super() failure is where
> the superblock is destroyed - nothing past that point could possibly
> be of any use.
> 
> I would still like the details on the problem you are seeing.

The Problem isn't produced by fill_super failure, instead it's produced 
by setup_bdev_super failure just a line before it. here is a snip from 
fs/super:

		error = setup_bdev_super(s, fc->sb_flags, fc);
		if (!error)
			error = fill_super(s, fc);
		if (error) {
			deactivate_locked_super(s);
			return error;
		}
and in the above code, fc->s_fs_info has already been transferred to sb 
as you have mentionned in the sget_fc() function before the above snip.
But subsequent calls after setup_bdev_super fail to free s_fs_info IIUC.


> 
> Normal operation (for filesystems that preallocate ->s_fs_info and hang
> it off fc) goes like this:
> 
> 	* fc->s_fs_info is allocated in ->init_fs_context()
> 	* it is modified (possibly) in ->parse_param()
> 	* eventually ->get_tree() is called and at some point it
> asks for superblock by calling sget_fc().  It may fail (in which
> case fc->s_fs_info stays where it is), if may return a preexisting
> superblock (ditto) *OR* it may create and return a new superblock.
> In that case fc->s_fs_info is no more - it's been moved over to
> sb->s_fs_info.  NULL is left behind.  From that point on the
> responsibility for that sucker is with the filesystem; nothing in
> VFS has any idea where to find it.
> 
In this case, it doesn create a new superblock which transferes the 
ownership of the pointer. But as i said the problem is that in the error 
path of setup_bdev_super(), there is no freeing of such memory and since 
the pointer has already been transfered and it's the responsibility is 
with the filesystem, put_fs_context() calling hfs_free_fc() doesn't free 
the allocated memory too.
> Again, there is no such thing as transferring it back to fc - once
> fill_super() has been called, there might be any number of additional
> things that need to be undone.
> 
As I said above, fill_super isn't even called in this case.
> For HFS I would expect that hfs_fill_super() would call hfs_mdb_put(sb)
> on all failures and have it called from subsequent ->put_super() if
> we succeed and later unmount the filesystem.  That seems to be where
> ->s_fs_info is taken out of superblock and freed.
> 
> What do you observe getting leaked and in which case does that happen?
> 
Exactly in bdev_file_open_by_dev() in the setup_bdev_super call 
mentionned above is what triggers the error path that doesn't free the 
hfs_sb_info since hfs_free_fc calls kfree on a NULL pointer..

Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

[kernel test robot]

Hello,

kernel test robot noticed "Oops:general_protection_fault,probably_for_non-canonical_address#:#[##]SMP_KASAN_PTI" on:

commit: 45f3d9974e382495db777e0290a32ba0cd6f454b ("[PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure")
url: https://github.com/intel-lab-lkp/linux/commits/Mehdi-Ben-Hadj-Khelifa/fs-super-fix-memory-leak-of-s_fs_info-on-setup_bdev_super-failure/20251115-001149
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 6da43bbeb6918164f7287269881a5f861ae09d7e
patch link: https://lore.kernel.org/all/20251114165255.101361-1-mehdi.benhadjkhelifa@gmail.com/
patch subject: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure

in testcase: nvml
version: nvml-x86_64-4cbe1fd37-1_20251013
with following parameters:

	test: non-pmem
	group: util



config: x86_64-rhel-9.4-func
compiler: gcc-14
test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz (Haswell) with 8G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202511262155.f86d1a5f-lkp@intel.com


[  164.783048][T42994] EXT4-fs (loop0): VFS: Can't find ext4 filesystem
[  164.792057][T42994] EXT4-fs (loop0): VFS: Can't find ext4 filesystem
[  164.798663][T42994] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
[  164.810433][T42994] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
[  164.818722][T42994] CPU: 3 UID: 0 PID: 42994 Comm: mount Tainted: G S                  6.18.0-rc5-00215-g45f3d9974e38 #1 PREEMPT(voluntary)
[  164.831362][T42994] Tainted: [S]=CPU_OUT_OF_SPEC
[  164.835992][T42994] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A05 12/05/2013
[  164.843927][T42994] RIP: 0010:fuse_kill_sb_blk (kbuild/src/consumer/fs/fuse/inode.c:2126 kbuild/src/consumer/fs/fuse/inode.c:2153) fuse
[  164.850056][T42994] Code: 00 00 00 00 00 fc ff df 48 c1 ea 03 80 3c 02 00 75 6a 48 8b 9b 90 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 da 48 c1 ea 03 <80> 3c 02 00 75 60 48 8b 3b e8 ec f8 ff ff 48 85 db 74 1a 48 83 c4
All code
========
   0:	00 00                	add    %al,(%rax)
   2:	00 00                	add    %al,(%rax)
   4:	00 fc                	add    %bh,%ah
   6:	ff                   	lcall  (bad)
   7:	df 48 c1             	fisttps -0x3f(%rax)
   a:	ea                   	(bad)
   b:	03 80 3c 02 00 75    	add    0x7500023c(%rax),%eax
  11:	6a 48                	push   $0x48
  13:	8b 9b 90 03 00 00    	mov    0x390(%rbx),%ebx
  19:	48 b8 00 00 00 00 00 	movabs $0xdffffc0000000000,%rax
  20:	fc ff df 
  23:	48 89 da             	mov    %rbx,%rdx
  26:	48 c1 ea 03          	shr    $0x3,%rdx
  2a:*	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)		<-- trapping instruction
  2e:	75 60                	jne    0x90
  30:	48 8b 3b             	mov    (%rbx),%rdi
  33:	e8 ec f8 ff ff       	call   0xfffffffffffff924
  38:	48 85 db             	test   %rbx,%rbx
  3b:	74 1a                	je     0x57
  3d:	48                   	rex.W
  3e:	83                   	.byte 0x83
  3f:	c4                   	.byte 0xc4

Code starting with the faulting instruction
===========================================
   0:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1)
   4:	75 60                	jne    0x66
   6:	48 8b 3b             	mov    (%rbx),%rdi
   9:	e8 ec f8 ff ff       	call   0xfffffffffffff8fa
   e:	48 85 db             	test   %rbx,%rbx
  11:	74 1a                	je     0x2d
  13:	48                   	rex.W
  14:	83                   	.byte 0x83
  15:	c4                   	.byte 0xc4
[  164.869568][T42994] RSP: 0018:ffffc900022dfbc8 EFLAGS: 00010246
[  164.875504][T42994] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff81580d23
[  164.883352][T42994] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff8882004c8014
[  164.891213][T42994] RBP: ffffffffc020dba0 R08: 0000000000000001 R09: ffffed1040099000
[  164.899068][T42994] R10: ffff8882004c8007 R11: ffffffff81e792d8 R12: 00000000ffffffea
[  164.906921][T42994] R13: ffff88810dc18390 R14: ffffffffc0446ab0 R15: 00000000ffffffea
[  164.914770][T42994] FS:  00007ff3e6309840(0000) GS:ffff88821483e000(0000) knlGS:0000000000000000
[  164.923579][T42994] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  164.930038][T42994] CR2: 000055c480d92328 CR3: 00000001ec872005 CR4: 00000000001726f0
[  164.937888][T42994] Call Trace:
[  164.941038][T42994]  <TASK>
[  164.943841][T42994]  ? __pfx_fuse_fill_super (kbuild/src/consumer/fs/fuse/inode.c:1939) fuse
[  164.949619][T42994]  deactivate_locked_super (kbuild/src/consumer/fs/super.c:434 kbuild/src/consumer/fs/super.c:475)
[  164.954861][T42994]  get_tree_bdev_flags (kbuild/src/consumer/fs/super.c:1699)
[  164.959839][T42994]  ? __pfx_get_tree_bdev_flags (kbuild/src/consumer/fs/super.c:1662)


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20251126/202511262155.f86d1a5f-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki