From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F31FB1F0E32 for ; Wed, 19 Nov 2025 22:21:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763590865; cv=none; b=TKQijzewXcKsN8/wzg1ddKOYCLTikiIbTKRWUKTiFb6won67xdgcjdDG/ucEAfA2SxJ+SseokMtfBxFvnF8Qv10tNuxNTbzPDXVX/Fppetx16/yzDmb8YcI6MrsST1bSU2cfKi5webh5ykWnfwQjeMsupUQ1imxBTWFLatOUpk8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763590865; c=relaxed/simple; bh=50Q9sBTjpmF9okPX4ahM+W4f4BYMCV2mUkt/5FtAic0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=jNovB0TvdomFWP1jp/ydZz9eppNFqgrwUbbqwFeqX3EDFvOyQhVJ3AE79mRrzcka7R2c6Mr5554F4bWmh4EnzLX+Rfqa1UMiCCoeNQOvdnaG1M+5PuXY9cdLdWxcMnOBI05aX4SgOq1kXg93f70bUHIy/pmuUWd8Q6m9HlVpB1g= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=JP0FCFJH; arc=none smtp.client-ip=209.85.128.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JP0FCFJH" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-47798f4059fso390115e9.2 for ; Wed, 19 Nov 2025 14:21:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763590862; x=1764195662; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=288Dw9sswG0A+z/lBkxnm7oNArScrtV4EVd6dIg2R2Y=; b=JP0FCFJHP2X5MXrAcMtQ+4dXgB7O2fhO5EEGwlNDXRVDWKtUvmuF82KPPc12AAfcGm doVt/W4Q/AYtsOQBjUtCtQ6gJUzNGTIWDhudlQpJKG56ZnAwZQmahr0wCQf6CP/haD7I 7BXQycyHTvPIBCLdaZvaiDLpI+GV5pV+0EYsFaFVlTdrI7OBiP3Gf4fd84scioTZqba6 y3TpcoRo1ASM9uo424amHjfUluJK6pGHd0oPs42osgsv42/adXv0osiqJGB7RBw8Oy1s Ens32drV7bvJduLuXLKA2Oxif1b3U5fRH4L3qxcAO42TvxA+BJ8jzCJo+f01+kCqdGeh fziw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763590862; x=1764195662; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=288Dw9sswG0A+z/lBkxnm7oNArScrtV4EVd6dIg2R2Y=; b=c0revgqoSuDNmBDiTNRue/vmENRFntYHy+SIKAKrsqUf/IsPaY7eTUEYgS9ML6CNR9 TsvveFtoq4jopjyWUwIzkwIpRDmTgEEt90OYHtMmqznw+sgNo9+FyRyz5rsJQd5rTcsR HQ2EARTF+06sQvFSRSVunyKvIs6689bswojJkaSJZ1m5F2bdhLCigZW+Cj1yx204ZWpg mEVEOnVcx0VSqrZGys8j/p5KmX13pU3uQ1vjwgMlAyogbblCYFv6CGppxihE4247OADW egKVKpsfEal8uS0O6LTQDfv1dZWbOd85H6Afrgltxvwn119dcht3glMGdyy/FE6ZYRt3 RHSg== X-Forwarded-Encrypted: i=1; AJvYcCWuQQUyXvTe/e9G3KWdQpgCkG5nbhIZd9h+qkp5LPmqoVpeuaG0+i6YXfOr+XAMGTWvi4i16mOjBkP8q9g=@vger.kernel.org X-Gm-Message-State: AOJu0YxmXipVcc/PLWNXgvU1LuSrFR5paav+B4fNthFKEGvdKa2oPwkI aqurKmzqc1q6V980N4xJSIkDGURKUcC5mP/zpx9Bo1WADIpYorspxOIB X-Gm-Gg: ASbGnctFFYHffEte8jKF9pPDvweBLcd3ALS1WPPPatpAZyZbGoeRLRxBqS2B2id/Z3r 42DhAC7H1OCXbtL2w3YZ0glJpALVZCkp2bZTif8c03G4MfsmTMCDXSbfrPCHO/BLeDfGX9iB46D SF28wDbEuPGMZ5uLTYslbofDolcP4HOCoQi6KNu1kdY9somooVA6yBIUwf9C2ALiplDGcsBw7JI iC4en5m+9OQXr/xrC4efABlz4AuKsXOt+lOPP0ODlXKp9NzWRzZdAXTpMCtsgVPPyBUaDnf9Ba7 ufSoh324MdEn39jzIUmceKQ3Ukm0IOjbTnEbGGwPf1w327bhahXrw5HI2YXOnYuM52eeNlLHnHP 4XEfCcdbFjuuf5KG/rj86Uy6wRqgJIO6/Bh127Ancq/We9XY7fZPI9MnR6CvqlxNVg6jo1wukgo tFdOD+3X3q5L+ryVljBQhaXhK4wAo2+295FIb7Lw== X-Google-Smtp-Source: AGHT+IG/wJ7Aaz37mXLzcO74yWwcjdbnF8mjOxzLT81DyT1s3fbL9Pty3G5JtiVp6En9+zLPVH7muw== X-Received: by 2002:a05:600c:4f0b:b0:477:a16e:fec5 with SMTP id 5b1f17b1804b1-477b8355ed8mr4060285e9.0.1763590862068; Wed, 19 Nov 2025 14:21:02 -0800 (PST) Received: from [192.168.1.111] ([165.50.70.6]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-477b106a9b0sm72950165e9.11.2025.11.19.14.20.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 19 Nov 2025 14:21:01 -0800 (PST) Message-ID: <25434098-4bf0-4330-b7b1-527983d9c903@gmail.com> Date: Wed, 19 Nov 2025 23:21:01 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super() failure To: Viacheslav Dubeyko , "glaubitz@physik.fu-berlin.de" , "frank.li@vivo.com" , "slava@dubeyko.com" , "brauner@kernel.org" , "viro@zeniv.linux.org.uk" , "jack@suse.cz" Cc: "khalid@kernel.org" , "linux-kernel-mentees@lists.linuxfoundation.org" , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "skhan@linuxfoundation.org" , "david.hunter.linux@gmail.com" , "syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com" References: <20251119073845.18578-1-mehdi.benhadjkhelifa@gmail.com> Content-Language: en-US From: Mehdi Ben Hadj Khelifa In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 11/19/25 8:58 PM, Viacheslav Dubeyko wrote: > On Wed, 2025-11-19 at 08:38 +0100, Mehdi Ben Hadj Khelifa wrote: >> The regression introduced by commit aca740cecbe5 ("fs: open block device >> after superblock creation") allows setup_bdev_super() to fail after a new >> superblock has been allocated by sget_fc(), but before hfs_fill_super() >> takes ownership of the filesystem-specific s_fs_info data. >> >> In that case, hfs_put_super() and the failure paths of hfs_fill_super() >> are never reached, leaving the HFS mdb structures attached to s->s_fs_info >> unreleased.The default kill_block_super() teardown also does not free >> HFS-specific resources, resulting in a memory leak on early mount failure. >> >> Fix this by moving all HFS-specific teardown (hfs_mdb_put()) from >> hfs_put_super() and the hfs_fill_super() failure path into a dedicated >> hfs_kill_sb() implementation. This ensures that both normal unmount and >> early teardown paths (including setup_bdev_super() failure) correctly >> release HFS metadata. >> >> This also preserves the intended layering: generic_shutdown_super() >> handles VFS-side cleanup, while HFS filesystem state is fully destroyed >> afterwards. >> >> Fixes: aca740cecbe5 ("fs: open block device after superblock creation") >> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6 >> Tested-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com >> Suggested-by: Al Viro >> Signed-off-by: Mehdi Ben Hadj Khelifa >> --- >> ChangeLog: >> >> Changes from v1: >> >> -Changed the patch direction to focus on hfs changes specifically as >> suggested by al viro >> >> Link:https://lore.kernel.org/all/20251114165255.101361-1-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 and test it with syzbot as well. > > Have you run xfstests for the patch? Unfortunately, we have multiple xfstests > failures for HFS now. And you can check the list of known issues here [1]. The > main point of such run of xfstests is to check that maybe some issue(s) could be > fixed by the patch. And, more important that you don't introduce new issues. ;) > I did not know of such tests. I will try to run them for both my patch and christian's patch[1] and report the results. >> >> fs/hfs/super.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/fs/hfs/super.c b/fs/hfs/super.c >> index 47f50fa555a4..06e1c25e47dc 100644 >> --- a/fs/hfs/super.c >> +++ b/fs/hfs/super.c >> @@ -49,8 +49,6 @@ static void hfs_put_super(struct super_block *sb) >> { >> cancel_delayed_work_sync(&HFS_SB(sb)->mdb_work); >> hfs_mdb_close(sb); >> - /* release the MDB's resources */ >> - hfs_mdb_put(sb); >> } >> >> static void flush_mdb(struct work_struct *work) >> @@ -383,7 +381,6 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc) >> bail_no_root: >> pr_err("get root inode failed\n"); >> bail: >> - hfs_mdb_put(sb); >> return res; >> } >> >> @@ -431,10 +428,21 @@ static int hfs_init_fs_context(struct fs_context *fc) >> return 0; >> } >> >> +static void hfs_kill_sb(struct super_block *sb) >> +{ >> + generic_shutdown_super(sb); >> + hfs_mdb_put(sb); >> + if (sb->s_bdev) { >> + sync_blockdev(sb->s_bdev); >> + bdev_fput(sb->s_bdev_file); >> + } >> + >> +} >> + >> static struct file_system_type hfs_fs_type = { >> .owner = THIS_MODULE, >> .name = "hfs", >> - .kill_sb = kill_block_super, > > It looks like we have the same issue for the case of HFS+ [2]. Could you please > double check that HFS+ should be fixed too? > Yes, I will check it tomorrow in addition to running xfstests and report my findings in response to this email. But I'm not sure if my solution would be the attended fix or a similar solution to what christian did is preferred instead for HFS+. We'll discuss it when I send a response. > Thanks, > Slava. > Thank you for your insights Slava! Best Regards, Mehdi Ben Hadj Khelifa >> + .kill_sb = hfs_kill_sb, >> .fs_flags = FS_REQUIRES_DEV, >> .init_fs_context = hfs_init_fs_context, >> }; > > [1] https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues > [2] https://elixir.bootlin.com/linux/v6.18-rc6/source/fs/hfsplus/super.c#L694