linux-kernel - Re: [RFC PATCH] fs/hfs: fix ABBA deadlock in hfs_mdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a2b8144a25206fba69e59e805d93c05444080132.camel@ibm.com>
Date: Tue, 13 Jan 2026 20:52:45 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
        "frank.li@...o.com" <frank.li@...o.com>,
        "wangjinchao600@...il.com"
	<wangjinchao600@...il.com>,
        "slava@...eyko.com" <slava@...eyko.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
CC: "syzbot+1e3ff4b07c16ca0f6fe2@...kaller.appspotmail.com"
	<syzbot+1e3ff4b07c16ca0f6fe2@...kaller.appspotmail.com>
Subject: Re:  [RFC PATCH] fs/hfs: fix ABBA deadlock in hfs_mdb_commit

On Tue, 2026-01-13 at 16:19 +0800, Jinchao Wang wrote:
> syzbot reported a hung task in hfs_mdb_commit where a deadlock occurs
> between the MDB buffer lock and the folio lock.
> 
> The deadlock happens because hfs_mdb_commit() holds the mdb_bh
> lock while calling sb_bread(), which attempts to acquire the lock
> on the same folio.

I don't quite to follow to your logic. We have only one sb_bread() [1] in
hfs_mdb_commit(). This read is trying to extract the volume bitmap. How is it
possible that superblock and volume bitmap is located at the same folio? Are you
sure? Which size of the folio do you imply here?

Also, it your logic is correct, then we never could be able to mount/unmount or
run any operations on HFS volumes because of likewise deadlock. However, I can
run xfstests on HFS volume.

> 
> thread1:
> hfs_mdb_commit
> 	->lock_buffer(HFS_SB(sb)->mdb_bh);
> 	->bh = sb_bread(sb, block);
> 		...->folio_lock(folio)
> 
> thread2:
> ->blkdev_writepages()
> 	->writeback_iter()
> 		->writeback_get_folio()
> 			->folio_lock(folio)
> 	->block_write_full_folio()
> 		__block_write_full_folio()
> 			->lock_buffer(bh)

The volume bitmap is metadata structure and it cannot be flushed like regular
user data blocks. So, I don't quite follow from your explanation how
hfs_mdb_commit() can be deadlocked with ->blkdev_writepages(). Currently, your
explanation and the fix motivation doesn't make sense to me completely. 

> 
> This patch removes the lock_buffer(mdb_bh) call. Since hfs_mdb_commit()
> is typically called via VFS paths where the superblock is already
> appropriately protected (e.g., during sync or unmount), the additional
> low-level buffer lock may be redundant and is the direct cause of the
> lock inversion.
> 

Even if you remove lock_buffer(mdb_bh), then, somehow, you are OK to keep
lock/unlock_buffer(HFS_SB(sb)->alt_mdb_bh). :) No, sorry, your fix is wrong and
I don't see how the picture that you are sharing could happen. I assume that you
have not correct understanding of the issue.

Which call trace do you have initially? What's the real problem that you are
trying to solve? The commit message doesn't contain any information about the
issue itself.

> I am seeking comments on whether this VFS-level protection is sufficient
> for HFS metadata consistency or if a more granular locking approach is
> preferred.
> 

This lock is HFS internal technique that implementing the protection of internal
file system operations. Sorry, but, currently, your point sounds completely
unreasonable.

Thanks,
Slava.

> Link: https://syzkaller.appspot.com/bug?extid=1e3ff4b07c16ca0f6fe2  
> Reported-by: syzbot <syzbot+1e3ff4b07c16ca0f6fe2@...kaller.appspotmail.com>
> 
> Signed-off-by: Jinchao Wang <wangjinchao600@...il.com>
> ---
>  fs/hfs/mdb.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
> index 53f3fae60217..c641adb94e6f 100644
> --- a/fs/hfs/mdb.c
> +++ b/fs/hfs/mdb.c
> @@ -268,7 +268,6 @@ void hfs_mdb_commit(struct super_block *sb)
>  	if (sb_rdonly(sb))
>  		return;
>  
> -	lock_buffer(HFS_SB(sb)->mdb_bh);
>  	if (test_and_clear_bit(HFS_FLG_MDB_DIRTY, &HFS_SB(sb)->flags)) {
>  		/* These parameters may have been modified, so write them back */
>  		mdb->drLsMod = hfs_mtime();
> @@ -340,7 +339,6 @@ void hfs_mdb_commit(struct super_block *sb)
>  			size -= len;
>  		}
>  	}
> -	unlock_buffer(HFS_SB(sb)->mdb_bh);
>  }
>  
>  void hfs_mdb_close(struct super_block *sb)

[1] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfs/mdb.c#L324