linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: brauner@kernel.org, jack@suse.cz,
	syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com,
	frank.li@vivo.com, glaubitz@physik.fu-berlin.de,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	slava@dubeyko.com, syzkaller-bugs@googlegroups.com,
	skhan@linuxfoundation.org, david.hunter.linux@gmail.com,
	khalid@kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH] fs/super: fix memory leak of s_fs_info on setup_bdev_super failure
Date: Tue, 18 Nov 2025 17:21:59 +0100	[thread overview]
Message-ID: <6c482108-78b8-4e09-814a-67820a5c021e@gmail.com> (raw)
In-Reply-To: <20251118145957.GD2441659@ZenIV>

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

  reply	other threads:[~2025-11-18 15:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c482108-78b8-4e09-814a-67820a5c021e@gmail.com \
    --to=mehdi.benhadjkhelifa@gmail.com \
    --cc=brauner@kernel.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=frank.li@vivo.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=jack@suse.cz \
    --cc=khalid@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=slava@dubeyko.com \
    --cc=syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).