* [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-13 4:27 [syzbot] [hfs?] memory leak in hfs_init_fs_context syzbot
@ 2025-11-14 5:12 ` Mehdi Ben Hadj Khelifa
2025-11-14 11:55 ` Christian Brauner
2025-11-19 13:43 ` Christian Brauner
0 siblings, 2 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-14 5:12 UTC (permalink / raw)
To: syzbot+ad45f827c88778ff7df6
Cc: frank.li, glaubitz, linux-fsdevel, linux-kernel, slava,
syzkaller-bugs, Mehdi Ben Hadj Khelifa
#syz test
diff --git a/fs/super.c b/fs/super.c
index 5bab94fb7e03..a99e5281b057 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 back sb_info ownership to fc to be freed 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 5:12 ` [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
@ 2025-11-14 11:55 ` Christian Brauner
2025-11-14 16:05 ` Mehdi Ben Hadj Khelifa
2025-11-14 17:15 ` Mehdi Ben Hadj Khelifa
2025-11-19 13:43 ` Christian Brauner
1 sibling, 2 replies; 13+ messages in thread
From: Christian Brauner @ 2025-11-14 11:55 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: syzbot+ad45f827c88778ff7df6, frank.li, glaubitz, linux-fsdevel,
linux-kernel, slava, syzkaller-bugs
On Fri, Nov 14, 2025 at 06:12:12AM +0100, Mehdi Ben Hadj Khelifa wrote:
> #syz test
>
> diff --git a/fs/super.c b/fs/super.c
> index 5bab94fb7e03..a99e5281b057 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 back sb_info ownership to fc to be freed 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
>
No, either free it in hfs_fill_super() when it fails or add a wrapper
around kill_block_super() for hfs and free it after ->kill_sb() has run.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 11:55 ` Christian Brauner
@ 2025-11-14 16:05 ` Mehdi Ben Hadj Khelifa
2025-11-14 17:15 ` Mehdi Ben Hadj Khelifa
1 sibling, 0 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-14 16:05 UTC (permalink / raw)
To: Christian Brauner
Cc: syzbot+ad45f827c88778ff7df6, frank.li, glaubitz, linux-fsdevel,
linux-kernel, slava, syzkaller-bugs
On 11/14/25 12:55 PM, Christian Brauner wrote:
> On Fri, Nov 14, 2025 at 06:12:12AM +0100, Mehdi Ben Hadj Khelifa wrote:
>> #syz test
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 5bab94fb7e03..a99e5281b057 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 back sb_info ownership to fc to be freed 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
>>
>
> No, either free it in hfs_fill_super() when it fails or add a wrapper
> around kill_block_super() for hfs and free it after ->kill_sb() has run.
Ah. I just saw your reply after my I just sent out a new similar test.
I will be working on it with your suggestion.
Best Regards,
Mehdi Ben Hadj Khelifa
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
@ 2025-11-14 16:52 Mehdi Ben Hadj Khelifa
2025-11-18 14:59 ` Al Viro
2025-11-26 14:01 ` kernel test robot
0 siblings, 2 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-14 16:52 UTC (permalink / raw)
To: viro, brauner, jack, syzbot+ad45f827c88778ff7df6
Cc: frank.li, glaubitz, linux-fsdevel, linux-kernel, slava,
syzkaller-bugs, skhan, david.hunter.linux, khalid,
linux-kernel-mentees, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 11:55 ` Christian Brauner
2025-11-14 16:05 ` Mehdi Ben Hadj Khelifa
@ 2025-11-14 17:15 ` Mehdi Ben Hadj Khelifa
1 sibling, 0 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-14 17:15 UTC (permalink / raw)
To: Christian Brauner
Cc: syzbot+ad45f827c88778ff7df6, frank.li, glaubitz, linux-fsdevel,
linux-kernel, slava, syzkaller-bugs
On 11/14/25 12:55 PM, Christian Brauner wrote:
> On Fri, Nov 14, 2025 at 06:12:12AM +0100, Mehdi Ben Hadj Khelifa wrote:
>> #syz test
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 5bab94fb7e03..a99e5281b057 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 back sb_info ownership to fc to be freed 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
>>
>
> No, either free it in hfs_fill_super() when it fails or add a wrapper
> around kill_block_super() for hfs and free it after ->kill_sb() has run.
Sorry for the noise,Resending with proper CCs:
I forgot to mention. I was giving back the ownership to the filesystem
context because upon setup_bdev_super fails put_fs_context still gets
called even if I would free s_fs_info in the kill_sb,so hfs_free_fc
would get a NULL pointer to kfree as a result..I don't think that would
be desirable.
I would be sending my patch out for more discussion.
Best Regards,
Mehdi Ben Hadj Khelifa
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 16:52 [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
@ 2025-11-18 14:59 ` Al Viro
2025-11-18 16:21 ` Mehdi Ben Hadj Khelifa
2025-11-26 14:01 ` kernel test robot
1 sibling, 1 reply; 13+ messages in thread
From: Al Viro @ 2025-11-18 14:59 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-18 14:59 ` Al Viro
@ 2025-11-18 16:21 ` Mehdi Ben Hadj Khelifa
2025-11-18 16:35 ` Al Viro
0 siblings, 1 reply; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-18 16:21 UTC (permalink / raw)
To: Al Viro
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-18 16:21 ` Mehdi Ben Hadj Khelifa
@ 2025-11-18 16:35 ` Al Viro
2025-11-18 16:55 ` Al Viro
2025-11-18 17:58 ` Mehdi Ben Hadj Khelifa
0 siblings, 2 replies; 13+ messages in thread
From: Al Viro @ 2025-11-18 16:35 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-18 16:35 ` Al Viro
@ 2025-11-18 16:55 ` Al Viro
2025-11-18 18:05 ` Mehdi Ben Hadj Khelifa
2025-11-18 17:58 ` Mehdi Ben Hadj Khelifa
1 sibling, 1 reply; 13+ messages in thread
From: Al Viro @ 2025-11-18 16:55 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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);
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-18 16:35 ` Al Viro
2025-11-18 16:55 ` Al Viro
@ 2025-11-18 17:58 ` Mehdi Ben Hadj Khelifa
1 sibling, 0 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-18 17:58 UTC (permalink / raw)
To: Al Viro
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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..
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-18 16:55 ` Al Viro
@ 2025-11-18 18:05 ` Mehdi Ben Hadj Khelifa
0 siblings, 0 replies; 13+ messages in thread
From: Mehdi Ben Hadj Khelifa @ 2025-11-18 18:05 UTC (permalink / raw)
To: Al Viro
Cc: brauner, jack, syzbot+ad45f827c88778ff7df6, frank.li, glaubitz,
linux-fsdevel, linux-kernel, slava, syzkaller-bugs, skhan,
david.hunter.linux, khalid, linux-kernel-mentees
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 5:12 ` [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
2025-11-14 11:55 ` Christian Brauner
@ 2025-11-19 13:43 ` Christian Brauner
1 sibling, 0 replies; 13+ messages in thread
From: Christian Brauner @ 2025-11-19 13:43 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: syzbot+ad45f827c88778ff7df6, frank.li, glaubitz, linux-fsdevel,
linux-kernel, slava, syzkaller-bugs
On Fri, Nov 14, 2025 at 06:12:12AM +0100, Mehdi Ben Hadj Khelifa wrote:
> #syz test
>
> diff --git a/fs/super.c b/fs/super.c
> index 5bab94fb7e03..a99e5281b057 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 back sb_info ownership to fc to be freed by put_fs_context()
> + */
> + fc->s_fs_info = s->s_fs_info;
> + s->s_fs_info = NULL;
> deactivate_locked_super(s);
> return error;
> }
#syz test: https://github.com/brauner/linux.git work.hfs.fixes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
2025-11-14 16:52 [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
2025-11-18 14:59 ` Al Viro
@ 2025-11-26 14:01 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-11-26 14:01 UTC (permalink / raw)
To: Mehdi Ben Hadj Khelifa
Cc: oe-lkp, lkp, linux-fsdevel, viro, brauner, jack,
syzbot+ad45f827c88778ff7df6, frank.li, glaubitz, linux-kernel,
slava, syzkaller-bugs, skhan, david.hunter.linux, khalid,
linux-kernel-mentees, Mehdi Ben Hadj Khelifa, oliver.sang
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-11-26 14:01 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 16:52 [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
2025-11-18 14:59 ` Al Viro
2025-11-18 16:21 ` Mehdi Ben Hadj Khelifa
2025-11-18 16:35 ` Al Viro
2025-11-18 16:55 ` Al Viro
2025-11-18 18:05 ` Mehdi Ben Hadj Khelifa
2025-11-18 17:58 ` Mehdi Ben Hadj Khelifa
2025-11-26 14:01 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2025-11-13 4:27 [syzbot] [hfs?] memory leak in hfs_init_fs_context syzbot
2025-11-14 5:12 ` [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure Mehdi Ben Hadj Khelifa
2025-11-14 11:55 ` Christian Brauner
2025-11-14 16:05 ` Mehdi Ben Hadj Khelifa
2025-11-14 17:15 ` Mehdi Ben Hadj Khelifa
2025-11-19 13:43 ` Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).