* Re: [PATCH 1/1] hfs: don't let internal B-tree inodes leak into the VFS open path
2026-09-16 9:09 ` [PATCH 1/1] hfs: don't let internal B-tree inodes leak into the VFS open path 1sh1ro
@ 2026-09-16 18:33 ` Viacheslav Dubeyko
0 siblings, 0 replies; 2+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-16 18:33 UTC (permalink / raw)
To: 1sh1ro, John Paul Adrian Glaubitz, Yangtao Li
Cc: linux-fsdevel, linux-kernel, Mateusz Guzik, Christian Brauner,
stable
On Wed, 2026-09-16 at 17:09 +0800, 1sh1ro wrote:
> hfs_btree_open() creates the extents and catalog B-tree inodes with
> iget_locked() and never assigns them a file type, so both stay cached
> with i_mode == 0 after mount.
>
> A crafted HFS image whose catalog contains a record whose DirID/FlNum
> aliases HFS_EXT_CNID (3) or HFS_CAT_CNID (4) makes hfs_lookup() ->
> hfs_iget() -> iget5_locked() return that already-cached internal
> inode
> (hfs_test_inode() only compares i_ino), without running
> hfs_read_inode(). The typeless inode then reaches the generic open
> path and, with CONFIG_DEBUG_VFS=y, deterministically panics:
>
> VFS_BUG_ON_INODE(!IS_ANON_FILE(inode)): inode:... fs:hfs mode:0
> opflags:0xc flags:0x0 state:0x0 count:2
> ------------[ cut here ]------------
> kernel BUG at fs/namei.c:4271!
> RIP: 0010:may_open+0x2ba/0x480 fs/namei.c:4271
> Call Trace:
> do_open fs/namei.c:4698 [inline]
> path_openat+0x1600/0x3de0 fs/namei.c:4863
> __x64_sys_openat+0x13f/0x1f0 fs/open.c:1385
>
> Fix it in two steps:
>
> - give the B-tree inodes an S_IFREG type in hfs_btree_open(), so the
> "every cached inode has a valid S_IFMT type" VFS invariant holds
> regardless of what is on disk;
>
> - refuse, in hfs_iget(), catalog records that address the reserved
> B-tree CNIDs so a malformed image cannot alias internal objects
> into dentries at all. Returning NULL follows the existing
> convention for unaddressable records: hfs_fill_super() turns it
> into a mount failure and hfs_lookup() into -EACCES.
>
> Without CONFIG_DEBUG_VFS the assertion compiles away and the same
> crafted image silently makes a typeless inode openable, so this is
> not
> merely a debug-config nuisance.
>
> Found by syzkaller-style fuzzing of v7.2-rc7; still reproducible on
> current mainline.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: 1sh1ro <yangguikun19@qq.com>
> ---
> fs/hfs/btree.c | 8 ++++++++
> fs/hfs/inode.c | 11 +++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 2eb37a2f64e8..bd69ace8f309 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -43,6 +43,14 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
> if (!tree->inode)
> goto free_tree;
> BUG_ON(!(inode_state_read_once(tree->inode) & I_NEW));
> +
> + /* B-tree inodes are internal objects that must never be exposed
> + * through directory entries. Give them a REG type so the VFS
> + * invariant "every cached inode has a valid S_IFMT type" holds
> even
> + * when a corrupted image contains catalog records aliasing these
> + * reserved CNIDs.
> + */
> + tree->inode->i_mode = S_IFREG;
In HFS+ we call hfsplus_iget() in hfs_btree_open() [1] and it assign
the S_IFREG in hfsplus_system_read_inode() [2]. But in HFS we call
iget_locked() instead. I think we need to call hfs_iget() there and to
have something like hfs_system_read_inode() that will assign S_IFREG to
the i_mode. But current fix doesn't look like right solution now. Could
you please explore this direction for the fix?
Thanks,
Slava.
> {
> struct hfs_mdb *mdb = HFS_SB(sb)->mdb;
> HFS_I(tree->inode)->flags = 0;
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index ac4a9055c5c0..bb759345f1fc 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -432,6 +432,17 @@ struct inode *hfs_iget(struct super_block *sb,
> struct hfs_cat_key *key, hfs_cat_
> default:
> return NULL;
> }
> +
> + /* Refuse catalog records that address the internal B-tree
> objects:
> + * a crafted image can otherwise make hfs_lookup() return the
> cached
> + * typeless B-tree inode, which then reaches may_open() with
> + * i_mode == 0 and trips VFS_BUG_ON_INODE() there. NULL follows
> the
> + * existing convention for unaddressable records (mount failure /
> + * -EACCES).
> + */
> + if (cnid == HFS_EXT_CNID || cnid == HFS_CAT_CNID)
> + return NULL;
> +
> inode = iget5_locked(sb, cnid, hfs_test_inode, hfs_read_inode,
> &data);
> if (inode && (inode_state_read_once(inode) & I_NEW))
> unlock_new_inode(inode);
> --
> 2.39.5
[1]
https://elixir.bootlin.com/linux/v7.3-rc3/source/fs/hfsplus/btree.c#L286
[2]
https://elixir.bootlin.com/linux/v7.3-rc3/source/fs/hfsplus/super.c#L60
^ permalink raw reply [flat|nested] 2+ messages in thread