Re: [PATCH 1/1] hfs: don't let internal B-tree inodes leak into the VFS open path

[Viacheslav Dubeyko]

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@xxxxxx>
> ---
>  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