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

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

[1sh1ro]

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;
    {
    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

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

[PATCH v2 0/2] hfs: keep the internal B-tree inodes out of the VFS open path

[1sh1ro]

Hi Viacheslav,

v2, following your suggestion.  hfs_btree_open() no longer patches i_mode
up by hand: there is now an hfs_system_iget()/hfs_system_read_inode()
pair, the HFS counterpart of hfsplus_iget()/hfsplus_system_read_inode().
The B-tree inodes are created through the generic inode-cache path and
get their S_IFREG there, and the extent-record validation still runs
before hfs_btree_open() succeeds.

hfs_iget() itself is keyed on a catalog record (key + rec) and the
B-tree inodes have no catalog record, so hfs_system_iget() is a sibling
entry point sharing the same inode-cache path.  Folding it into
hfs_iget() is a small change if you prefer that shape.

Patch 2 keeps the alias guard from v1 - a catalog record must not address
CNID 3/4 - as hardening on top of patch 1.  Drop it if you consider
hfs_is_valid_cnid()'s acceptance of HFS_CDR_FIL records for those CNIDs
intentional.

The series applies to v7.3-rc5 and fs/hfs builds clean with clang.

Thanks,
1sh1ro

1sh1ro (2):
  hfs: create the internal B-tree inodes through the generic iget path
  hfs: refuse catalog records that address the reserved B-tree CNIDs

 fs/hfs/btree.c  |  25 +++++--------------------
 fs/hfs/hfs_fs.h |   1 +
 fs/hfs/inode.c  |  85 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 20 deletions(-)

[PATCH v2 2/2] hfs: refuse catalog records that address the reserved B-tree CNIDs

[1sh1ro]

The extent and catalog B-tree inodes are described by the MDB, not by
catalog records, but they are hashed by CNID like any other inode and
hfs_test_inode() compares i_ino only.  A crafted image with a catalog
record whose FlNum (or DirID) is HFS_EXT_CNID or HFS_CAT_CNID therefore
makes hfs_lookup() hand out the internal B-tree inode as a regular dentry,
bypassing the CNID validation that hfs_read_inode() performs: a read
exposes the B-tree file's contents, and a write would go through
hfs_write_inode(), which tries to update a catalog record that does not
exist.

Refuse such records in hfs_iget() by returning NULL for the reserved
B-tree CNIDs.  hfs_lookup() reports that as -EACCES and
hfs_fill_super() as a mount failure, the same as for any other
unaddressable record.

This is hardening on top of the file-type fix, not required for the
panic; it can be dropped if hfs_is_valid_cnid()'s acceptance of
HFS_CDR_FIL records for CNID 3/4 is intentional.

Signed-off-by: 1sh1ro <yangguikun19@qq.com>
---
  fs/hfs/inode.c |  11 +++++++++++
 1 file changed, 11 insertions(+), 0 deletions(-)

diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 4316d75..5190856 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -520,6 +520,17 @@ struct inode *hfs_iget(struct super_block *sb, struct hfs_cat_key *key, hfs_cat_
 	default:
 		return NULL;
 	}
+
+	/*
+	 * Never let a catalog record address one of the internal B-tree
+	 * objects.  hfs_test_inode() matches on i_ino alone, so such a
+	 * record would make hfs_lookup() hand out the cached B-tree inode as
+	 * a regular dentry, bypassing the CNID validation done by
+	 * hfs_read_inode().
+	 */
+	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

Re: [PATCH v2 2/2] hfs: refuse catalog records that address the reserved B-tree CNIDs

[Viacheslav Dubeyko]

On Mon, 2026-10-12 at 15:44 +0800, 1sh1ro wrote:
> The extent and catalog B-tree inodes are described by the MDB, not by
> catalog records, but they are hashed by CNID like any other inode and
> hfs_test_inode() compares i_ino only.  A crafted image with a catalog
> record whose FlNum (or DirID) is HFS_EXT_CNID or HFS_CAT_CNID
> therefore
> makes hfs_lookup() hand out the internal B-tree inode as a regular
> dentry,
> bypassing the CNID validation that hfs_read_inode() performs: a read
> exposes the B-tree file's contents, and a write would go through
> hfs_write_inode(), which tries to update a catalog record that does
> not
> exist.
> 
> Refuse such records in hfs_iget() by returning NULL for the reserved
> B-tree CNIDs.  hfs_lookup() reports that as -EACCES and
> hfs_fill_super() as a mount failure, the same as for any other
> unaddressable record.
> 
> This is hardening on top of the file-type fix, not required for the
> panic; it can be dropped if hfs_is_valid_cnid()'s acceptance of
> HFS_CDR_FIL records for CNID 3/4 is intentional.

Please, combine both patches into one. First patch doesn't fix the
issue alone.

> 
> Signed-off-by: 1sh1ro <yangguikun19@qq.com>

Please, provide the real name.

> ---
>   fs/hfs/inode.c |  11 +++++++++++
>  1 file changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 4316d75..5190856 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -520,6 +520,17 @@ struct inode *hfs_iget(struct super_block *sb,
> struct hfs_cat_key *key, hfs_cat_
>  	default:
>  		return NULL;
>  	}
> +
> +	/*
> +	 * Never let a catalog record address one of the internal B-
> tree
> +	 * objects.  hfs_test_inode() matches on i_ino alone, so
> such a
> +	 * record would make hfs_lookup() hand out the cached B-tree
> inode as
> +	 * a regular dentry, bypassing the CNID validation done by
> +	 * hfs_read_inode().
> +	 */

Please, rework or remove the comment. Currently, it's long and not
informative.

> +	if (cnid == HFS_EXT_CNID || cnid == HFS_CAT_CNID)

I think we can reject all cnid that lower than HFS_FIRSTUSER_CNID.

Thanks,
Slava.

> +		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);

[PATCH v3] hfs: give the internal B-tree inodes a type and keep catalog records off them

[yangguikun]

hfs_btree_open() builds the extents and catalog B-tree inodes with a raw
iget_locked() and never assigns them a file type, so both stay cached with
i_mode == 0 after every mount.  Two problems follow from that, and both are
reachable from a crafted image.

First, a catalog record that names CNID 3 or 4 makes hfs_lookup() -> hfs_iget()
return the cached B-tree inode - hfs_test_inode() matches on i_ino alone - and
the typeless inode then reaches the generic open path, where it trips
VFS_BUG_ON_INODE() in may_open() with CONFIG_DEBUG_VFS=y:

  VFS_BUG_ON_INODE(!IS_ANON_FILE(inode)): inode:... fs:hfs mode:0 opflags:0xc
  ------------[ 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

Without CONFIG_DEBUG_VFS the assertion compiles away and the same image
silently yields an openable typeless inode, so this is not merely a
debug-config nuisance.

Second, once the inode has a type, such a record still hands the internal
B-tree file out as a regular dentry: it can be opened and read, which exposes
the B-tree, and from then on the VFS treats an inode that has no catalog
record behind it as an ordinary file.  hfs_is_valid_cnid() does not close this
path: it runs from hfs_read_inode(), and a lookup that hits the already cached
B-tree inode never reaches that function.

Fix both.  Add hfs_system_read_inode(), which reads the fork from the MDB and
assigns S_IFREG - the counterpart of hfsplus_system_read_inode() - and
hfs_system_iget(), which looks the inode up through the normal inode-cache path
and can fail before the inode is published.  hfs_btree_open() now uses
hfs_system_iget(), and the extent-record validation moves into the new read
helper.  hfs_iget() refuses catalog records that name a reserved CNID below
HFS_FIRSTUSER_CNID; the root directory, the one reserved CNID that legitimately
comes through there, is exempt.

Found by syzkaller-style fuzzing of v7.2-rc7; still reproducible on current
mainline.

Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/linux-fsdevel/tencent_24D928451FEFA8A35482EFFA9132A7305307@qq.com/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: yangguikun <yangguikun19@qq.com>
---
 fs/hfs/btree.c  | 39 +++------------------------
 fs/hfs/hfs_fs.h |  1 +
 fs/hfs/inode.c  | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 36 deletions(-)

diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
index 41b4e8f..b5892de 100644
--- a/fs/hfs/btree.c
+++ b/fs/hfs/btree.c
@@ -155,44 +155,11 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id, btree_keycmp ke
 	tree->cnid = id;
 	tree->keycmp = keycmp;
 
-	tree->inode = iget_locked(sb, id);
-	if (!tree->inode)
+	tree->inode = hfs_system_iget(sb, id);
+	if (IS_ERR(tree->inode))
 		goto free_tree;
-	BUG_ON(!(inode_state_read_once(tree->inode) & I_NEW));
-	{
-	struct hfs_mdb *mdb = HFS_SB(sb)->mdb;
-	HFS_I(tree->inode)->flags = 0;
-	mutex_init(&HFS_I(tree->inode)->extents_lock);
-	switch (id) {
-	case HFS_EXT_CNID:
-		hfs_inode_read_fork(tree->inode, mdb->drXTExtRec, mdb->drXTFlSize,
-				    mdb->drXTFlSize, be32_to_cpu(mdb->drXTClpSiz));
-		if (HFS_I(tree->inode)->alloc_blocks >
-					HFS_I(tree->inode)->first_blocks) {
-			pr_err("invalid btree extent records\n");
-			unlock_new_inode(tree->inode);
-			goto free_inode;
-		}
 
-		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
-		break;
-	case HFS_CAT_CNID:
-		hfs_inode_read_fork(tree->inode, mdb->drCTExtRec, mdb->drCTFlSize,
-				    mdb->drCTFlSize, be32_to_cpu(mdb->drCTClpSiz));
-
-		if (!HFS_I(tree->inode)->first_blocks) {
-			pr_err("invalid btree extent records (0 size)\n");
-			unlock_new_inode(tree->inode);
-			goto free_inode;
-		}
-
-		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
-		break;
-	default:
-		BUG();
-	}
-	}
-	unlock_new_inode(tree->inode);
+	tree->inode->i_mapping->a_ops = &hfs_btree_aops;
 
 	mapping = tree->inode->i_mapping;
 	folio = filemap_grab_folio(mapping, 0);
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index e250f87..26505f7 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -219,6 +219,7 @@ extern void hfs_inode_read_fork(struct inode *inode, struct hfs_extent *ext,
 				u32 clump_size);
 extern struct inode *hfs_iget(struct super_block *sb, struct hfs_cat_key *key,
 				hfs_cat_rec *rec);
+extern struct inode *hfs_system_iget(struct super_block *sb, u32 cnid);
 extern void hfs_evict_inode(struct inode *inode);
 extern void hfs_delete_inode(struct inode *inode);
 
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 2aef3c3..5c00383 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -421,6 +421,87 @@ static int hfs_read_inode(struct inode *inode, void *data)
 	return 0;
 }
 
+/**
+ * hfs_system_read_inode - read an internal B-tree inode
+ * @inode: inode to initialize, hashed by its CNID
+ *
+ * The extent and catalog B-trees are described by the MDB instead of by
+ * catalog records, so they do not go through hfs_read_inode().
+ *
+ * Return: 0 on success, -EIO if the MDB does not describe this CNID.
+ */
+static int hfs_system_read_inode(struct inode *inode)
+{
+	struct hfs_sb_info *hsb = HFS_SB(inode->i_sb);
+	struct hfs_mdb *mdb = hsb->mdb;
+
+	HFS_I(inode)->flags = 0;
+	HFS_I(inode)->rsrc_inode = NULL;
+	mutex_init(&HFS_I(inode)->extents_lock);
+	HFS_I(inode)->tz_secondswest = sys_tz.tz_minuteswest * 60;
+
+	inode->i_uid = hsb->s_uid;
+	inode->i_gid = hsb->s_gid;
+	set_nlink(inode, 1);
+
+	switch (inode->i_ino) {
+	case HFS_EXT_CNID:
+		hfs_inode_read_fork(inode, mdb->drXTExtRec, mdb->drXTFlSize,
+				    mdb->drXTFlSize, be32_to_cpu(mdb->drXTClpSiz));
+		if (HFS_I(inode)->alloc_blocks > HFS_I(inode)->first_blocks) {
+			pr_err("invalid btree extent records\n");
+			return -EIO;
+		}
+		break;
+	case HFS_CAT_CNID:
+		hfs_inode_read_fork(inode, mdb->drCTExtRec, mdb->drCTFlSize,
+				    mdb->drCTFlSize, be32_to_cpu(mdb->drCTClpSiz));
+		if (!HFS_I(inode)->first_blocks) {
+			pr_err("invalid btree extent records (0 size)\n");
+			return -EIO;
+		}
+		break;
+	default:
+		return -EIO;
+	}
+
+	/*
+	 * may_open() requires a file type, and a crafted catalog record can
+	 * reach this inode by its CNID.
+	 */
+	inode->i_mode = S_IFREG;
+
+	return 0;
+}
+
+/**
+ * hfs_system_iget - look up an internal B-tree inode by CNID
+ * @sb: super block
+ * @cnid: HFS_EXT_CNID or HFS_CAT_CNID
+ *
+ * Return: the inode, or an ERR_PTR() if it cannot be initialized.
+ */
+struct inode *hfs_system_iget(struct super_block *sb, u32 cnid)
+{
+	struct inode *inode;
+	int err;
+
+	inode = iget_locked(sb, cnid);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+	if (!(inode_state_read_once(inode) & I_NEW))
+		return inode;
+
+	err = hfs_system_read_inode(inode);
+	if (err) {
+		iget_failed(inode);
+		return ERR_PTR(err);
+	}
+
+	unlock_new_inode(inode);
+	return inode;
+}
+
 /*
  * __hfs_iget()
  *
@@ -446,6 +527,15 @@ struct inode *hfs_iget(struct super_block *sb, struct hfs_cat_key *key, hfs_cat_
 	default:
 		return NULL;
 	}
+
+	/*
+	 * hfs_test_inode() matches on i_ino only, so a catalog record naming
+	 * a reserved CNID would hand out a B-tree inode as a regular dentry.
+	 * The root directory is the one reserved CNID that comes through here.
+	 */
+	if (cnid != HFS_ROOT_CNID && cnid < HFS_FIRSTUSER_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

Re: [PATCH v3] hfs: give the internal B-tree inodes a type and keep catalog records off them

[Viacheslav Dubeyko]

On Tue, 2026-10-13 at 14:44 +0800, yangguikun wrote:
> hfs_btree_open() builds the extents and catalog B-tree inodes with a
> raw
> iget_locked() and never assigns them a file type, so both stay cached
> with
> i_mode == 0 after every mount.  Two problems follow from that, and
> both are
> reachable from a crafted image.
> 
> First, a catalog record that names CNID 3 or 4 makes hfs_lookup() ->
> hfs_iget()
> return the cached B-tree inode - hfs_test_inode() matches on i_ino
> alone - and
> the typeless inode then reaches the generic open path, where it trips
> VFS_BUG_ON_INODE() in may_open() with CONFIG_DEBUG_VFS=y:
> 
>   VFS_BUG_ON_INODE(!IS_ANON_FILE(inode)): inode:... fs:hfs mode:0
> opflags:0xc
>   ------------[ 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
> 
> Without CONFIG_DEBUG_VFS the assertion compiles away and the same
> image
> silently yields an openable typeless inode, so this is not merely a
> debug-config nuisance.
> 
> Second, once the inode has a type, such a record still hands the
> internal
> B-tree file out as a regular dentry: it can be opened and read, which
> exposes
> the B-tree, and from then on the VFS treats an inode that has no
> catalog
> record behind it as an ordinary file.  hfs_is_valid_cnid() does not
> close this
> path: it runs from hfs_read_inode(), and a lookup that hits the
> already cached
> B-tree inode never reaches that function.
> 
> Fix both.  Add hfs_system_read_inode(), which reads the fork from the
> MDB and
> assigns S_IFREG - the counterpart of hfsplus_system_read_inode() -
> and
> hfs_system_iget(), which looks the inode up through the normal inode-
> cache path
> and can fail before the inode is published.  hfs_btree_open() now
> uses
> hfs_system_iget(), and the extent-record validation moves into the
> new read
> helper.  hfs_iget() refuses catalog records that name a reserved CNID
> below
> HFS_FIRSTUSER_CNID; the root directory, the one reserved CNID that
> legitimately
> comes through there, is exempt.
> 
> Found by syzkaller-style fuzzing of v7.2-rc7; still reproducible on
> current
> mainline.
> 
> Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
> Link:
> https://lore.kernel.org/linux-fsdevel/tencent_24D928451FEFA8A35482EFFA9132A7305307@qq.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: yangguikun <yangguikun19@qq.com>
> ---
>  fs/hfs/btree.c  | 39 +++------------------------
>  fs/hfs/hfs_fs.h |  1 +
>  fs/hfs/inode.c  | 82
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 41b4e8f..b5892de 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -155,44 +155,11 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
>  	tree->cnid = id;
>  	tree->keycmp = keycmp;
>  
> -	tree->inode = iget_locked(sb, id);
> -	if (!tree->inode)
> +	tree->inode = hfs_system_iget(sb, id);
> +	if (IS_ERR(tree->inode))
>  		goto free_tree;

I worried that we jump to free_tree now. Before, we cannot get inode
but now method can or cannot get inode. We need somehow distinguish
these cases now and to jump to free_tree or to free_inode. Or we need
to manage it in more smart way.

> -	BUG_ON(!(inode_state_read_once(tree->inode) & I_NEW));

I am not sure that it's good idea to drop this. If hfs_system_iget()
quietly returns an existing inode. In hfs_btree_open() that would mean
the same tree was opened twice.

> -	{
> -	struct hfs_mdb *mdb = HFS_SB(sb)->mdb;
> -	HFS_I(tree->inode)->flags = 0;
> -	mutex_init(&HFS_I(tree->inode)->extents_lock);
> -	switch (id) {
> -	case HFS_EXT_CNID:
> -		hfs_inode_read_fork(tree->inode, mdb->drXTExtRec,
> mdb->drXTFlSize,
> -				    mdb->drXTFlSize,
> be32_to_cpu(mdb->drXTClpSiz));
> -		if (HFS_I(tree->inode)->alloc_blocks >
> -					HFS_I(tree->inode)-
> >first_blocks) {
> -			pr_err("invalid btree extent records\n");
> -			unlock_new_inode(tree->inode);
> -			goto free_inode;
> -		}
>  
> -		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
> -		break;
> -	case HFS_CAT_CNID:
> -		hfs_inode_read_fork(tree->inode, mdb->drCTExtRec,
> mdb->drCTFlSize,
> -				    mdb->drCTFlSize,
> be32_to_cpu(mdb->drCTClpSiz));
> -
> -		if (!HFS_I(tree->inode)->first_blocks) {
> -			pr_err("invalid btree extent records (0
> size)\n");
> -			unlock_new_inode(tree->inode);
> -			goto free_inode;
> -		}
> -
> -		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
> -		break;
> -	default:
> -		BUG();
> -	}
> -	}
> -	unlock_new_inode(tree->inode);
> +	tree->inode->i_mapping->a_ops = &hfs_btree_aops;

 Set a_ops inside hfs_system_read_inode(). Right now tree->inode-
>i_mapping->a_ops = &hfs_btree_aops; runs in hfs_btree_open() after
unlock_new_inode(), so the inode is visible in the hash briefly without
its aops.

>  
>  	mapping = tree->inode->i_mapping;
>  	folio = filemap_grab_folio(mapping, 0);
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index e250f87..26505f7 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -219,6 +219,7 @@ extern void hfs_inode_read_fork(struct inode
> *inode, struct hfs_extent *ext,
>  				u32 clump_size);
>  extern struct inode *hfs_iget(struct super_block *sb, struct
> hfs_cat_key *key,
>  				hfs_cat_rec *rec);
> +extern struct inode *hfs_system_iget(struct super_block *sb, u32
> cnid);
>  extern void hfs_evict_inode(struct inode *inode);
>  extern void hfs_delete_inode(struct inode *inode);
>  
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 2aef3c3..5c00383 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -421,6 +421,87 @@ static int hfs_read_inode(struct inode *inode,
> void *data)
>  	return 0;
>  }
>  
> +/**
> + * hfs_system_read_inode - read an internal B-tree inode
> + * @inode: inode to initialize, hashed by its CNID
> + *
> + * The extent and catalog B-trees are described by the MDB instead
> of by
> + * catalog records, so they do not go through hfs_read_inode().
> + *
> + * Return: 0 on success, -EIO if the MDB does not describe this
> CNID.
> + */
> +static int hfs_system_read_inode(struct inode *inode)
> +{
> +	struct hfs_sb_info *hsb = HFS_SB(inode->i_sb);
> +	struct hfs_mdb *mdb = hsb->mdb;
> +
> +	HFS_I(inode)->flags = 0;
> +	HFS_I(inode)->rsrc_inode = NULL;
> +	mutex_init(&HFS_I(inode)->extents_lock);
> +	HFS_I(inode)->tz_secondswest = sys_tz.tz_minuteswest * 60;
> +
> +	inode->i_uid = hsb->s_uid;
> +	inode->i_gid = hsb->s_gid;
> +	set_nlink(inode, 1);
> +
> +	switch (inode->i_ino) {
> +	case HFS_EXT_CNID:
> +		hfs_inode_read_fork(inode, mdb->drXTExtRec, mdb-
> >drXTFlSize,
> +				    mdb->drXTFlSize,
> be32_to_cpu(mdb->drXTClpSiz));
> +		if (HFS_I(inode)->alloc_blocks > HFS_I(inode)-
> >first_blocks) {
> +			pr_err("invalid btree extent records\n");
> +			return -EIO;
> +		}
> +		break;
> +	case HFS_CAT_CNID:
> +		hfs_inode_read_fork(inode, mdb->drCTExtRec, mdb-
> >drCTFlSize,
> +				    mdb->drCTFlSize,
> be32_to_cpu(mdb->drCTClpSiz));
> +		if (!HFS_I(inode)->first_blocks) {
> +			pr_err("invalid btree extent records (0
> size)\n");
> +			return -EIO;
> +		}
> +		break;
> +	default:
> +		return -EIO;
> +	}
> +
> +	/*
> +	 * may_open() requires a file type, and a crafted catalog
> record can
> +	 * reach this inode by its CNID.
> +	 */
> +	inode->i_mode = S_IFREG;
> +
> +	return 0;
> +}
> +
> +/**
> + * hfs_system_iget - look up an internal B-tree inode by CNID
> + * @sb: super block
> + * @cnid: HFS_EXT_CNID or HFS_CAT_CNID
> + *
> + * Return: the inode, or an ERR_PTR() if it cannot be initialized.
> + */
> +struct inode *hfs_system_iget(struct super_block *sb, u32 cnid)
> +{
> +	struct inode *inode;
> +	int err;
> +
> +	inode = iget_locked(sb, cnid);
> +	if (!inode)
> +		return ERR_PTR(-ENOMEM);
> +	if (!(inode_state_read_once(inode) & I_NEW))
> +		return inode;
> +
> +	err = hfs_system_read_inode(inode);
> +	if (err) {
> +		iget_failed(inode);
> +		return ERR_PTR(err);
> +	}
> +
> +	unlock_new_inode(inode);
> +	return inode;
> +}
> +
>  /*
>   * __hfs_iget()
>   *
> @@ -446,6 +527,15 @@ struct inode *hfs_iget(struct super_block *sb,
> struct hfs_cat_key *key, hfs_cat_
>  	default:
>  		return NULL;
>  	}
> +
> +	/*
> +	 * hfs_test_inode() matches on i_ino only, so a catalog
> record naming
> +	 * a reserved CNID would hand out a B-tree inode as a
> regular dentry.
> +	 * The root directory is the one reserved CNID that comes
> through here.
> +	 */
> +	if (cnid != HFS_ROOT_CNID && cnid < HFS_FIRSTUSER_CNID)

I am guessing maybe we need to introduce a special method:

static inline bool hfs_is_system_reserved_cnid(u32 cnid)
{
	if (likely(cnid > HFS_ROOT_CNID	&& cnid <
HFS_FIRSTUSER_CNID))
		return true;
	else
        	return false;
}

What do you think?

Thanks,
Slava.

> +		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);

[PATCH v2 1/2] hfs: create the internal B-tree inodes through the generic iget path

[1sh1ro]

hfs_btree_open() builds the extents and catalog B-tree inodes with a raw
iget_locked() and never assigns them a file type, so both stay cached with
i_mode == 0 after every mount.  A crafted HFS image whose catalog contains
a record addressing one of the reserved B-tree CNIDs (3 or 4) makes
hfs_lookup() -> hfs_iget() return that cached inode - hfs_test_inode()
matches on i_ino alone - and the typeless inode then reaches the generic
open path, where it trips VFS_BUG_ON_INODE() in may_open() with
CONFIG_DEBUG_VFS=y:

  VFS_BUG_ON_INODE(!IS_ANON_FILE(inode)): inode:... fs:hfs mode:0 opflags:0xc
  ------------[ 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

Without CONFIG_DEBUG_VFS the assertion compiles away and the same image
silently yields an openable typeless inode, so this is not merely a
debug-config nuisance.

Fix it the way HFS+ handles its system inodes: add hfs_system_read_inode(),
which reads the fork from the MDB (the counterpart of
hfsplus_system_read_inode()), and hfs_system_iget(), which looks the inode
up through the normal inode-cache path and can fail before the inode is
published.  hfs_btree_open() now uses hfs_system_iget(), so the B-tree
inodes carry a valid S_IFREG type like every other inode.  The
extent-record validation is unchanged; it just runs after the inode has
been created.

Found by syzkaller-style fuzzing of v7.2-rc7; still reproducible on
current mainline.

Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/linux-fsdevel/tencent_24D928451FEFA8A35482EFFA9132A7305307@qq.com/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: 1sh1ro <yangguikun19@qq.com>
---
  fs/hfs/btree.c  |  25 +++++--------------------
 fs/hfs/hfs_fs.h |   1 +
 fs/hfs/inode.c  |  74 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 20 deletions(-)

diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
index 41b4e8f..7486707 100644
--- a/fs/hfs/btree.c
+++ b/fs/hfs/btree.c
@@ -155,44 +155,29 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id, btree_keycmp ke
 	tree->cnid = id;
 	tree->keycmp = keycmp;
 
-	tree->inode = iget_locked(sb, id);
-	if (!tree->inode)
+	tree->inode = hfs_system_iget(sb, id);
+	if (IS_ERR(tree->inode))
 		goto free_tree;
-	BUG_ON(!(inode_state_read_once(tree->inode) & I_NEW));
-	{
-	struct hfs_mdb *mdb = HFS_SB(sb)->mdb;
-	HFS_I(tree->inode)->flags = 0;
-	mutex_init(&HFS_I(tree->inode)->extents_lock);
+
 	switch (id) {
 	case HFS_EXT_CNID:
-		hfs_inode_read_fork(tree->inode, mdb->drXTExtRec, mdb->drXTFlSize,
-				    mdb->drXTFlSize, be32_to_cpu(mdb->drXTClpSiz));
 		if (HFS_I(tree->inode)->alloc_blocks >
 					HFS_I(tree->inode)->first_blocks) {
 			pr_err("invalid btree extent records\n");
-			unlock_new_inode(tree->inode);
 			goto free_inode;
 		}
-
-		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
 		break;
 	case HFS_CAT_CNID:
-		hfs_inode_read_fork(tree->inode, mdb->drCTExtRec, mdb->drCTFlSize,
-				    mdb->drCTFlSize, be32_to_cpu(mdb->drCTClpSiz));
-
 		if (!HFS_I(tree->inode)->first_blocks) {
 			pr_err("invalid btree extent records (0 size)\n");
-			unlock_new_inode(tree->inode);
 			goto free_inode;
 		}
-
-		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
 		break;
 	default:
 		BUG();
 	}
-	}
-	unlock_new_inode(tree->inode);
+
+	tree->inode->i_mapping->a_ops = &hfs_btree_aops;
 
 	mapping = tree->inode->i_mapping;
 	folio = filemap_grab_folio(mapping, 0);
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index e250f87..26505f7 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -219,6 +219,7 @@ extern void hfs_inode_read_fork(struct inode *inode, struct hfs_extent *ext,
 				u32 clump_size);
 extern struct inode *hfs_iget(struct super_block *sb, struct hfs_cat_key *key,
 				hfs_cat_rec *rec);
+extern struct inode *hfs_system_iget(struct super_block *sb, u32 cnid);
 extern void hfs_evict_inode(struct inode *inode);
 extern void hfs_delete_inode(struct inode *inode);
 
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 2aef3c3..4316d75 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -421,6 +421,80 @@ static int hfs_read_inode(struct inode *inode, void *data)
 	return 0;
 }
 
+/*
+ * hfs_system_read_inode
+ *
+ * Initialize one of the internal B-tree inodes.  These are described by the
+ * MDB rather than by catalog records, so they need their own read helper -
+ * the HFS counterpart of hfsplus_system_read_inode().
+ */
+static int hfs_system_read_inode(struct inode *inode)
+{
+	struct hfs_sb_info *hsb = HFS_SB(inode->i_sb);
+	struct hfs_mdb *mdb = hsb->mdb;
+
+	HFS_I(inode)->flags = 0;
+	HFS_I(inode)->rsrc_inode = NULL;
+	mutex_init(&HFS_I(inode)->extents_lock);
+	HFS_I(inode)->tz_secondswest = sys_tz.tz_minuteswest * 60;
+
+	inode->i_uid = hsb->s_uid;
+	inode->i_gid = hsb->s_gid;
+	set_nlink(inode, 1);
+
+	switch (inode->i_ino) {
+	case HFS_EXT_CNID:
+		hfs_inode_read_fork(inode, mdb->drXTExtRec, mdb->drXTFlSize,
+				    mdb->drXTFlSize, be32_to_cpu(mdb->drXTClpSiz));
+		break;
+	case HFS_CAT_CNID:
+		hfs_inode_read_fork(inode, mdb->drCTExtRec, mdb->drCTFlSize,
+				    mdb->drCTFlSize, be32_to_cpu(mdb->drCTClpSiz));
+		break;
+	default:
+		return -EIO;
+	}
+
+	/*
+	 * Assign a dummy file type, for may_open() requires that an inode has
+	 * a valid file type.  These inodes are hashed by CNID like any other
+	 * inode, so without a type a catalog record of a crafted image that
+	 * aliases CNID 3 or 4 would reach the VFS with i_mode == 0.
+	 */
+	inode->i_mode = S_IFREG;
+
+	return 0;
+}
+
+/*
+ * hfs_system_iget
+ *
+ * Look up (or create) one of the internal B-tree inodes.  hfs_btree_open()
+ * uses this instead of a raw iget_locked() so that the inode is completely
+ * initialized before it becomes visible in the inode cache, in the same way
+ * hfsplus_iget() handles the system CNIDs for HFS+.
+ */
+struct inode *hfs_system_iget(struct super_block *sb, u32 cnid)
+{
+	struct inode *inode;
+	int err;
+
+	inode = iget_locked(sb, cnid);
+	if (!inode)
+		return ERR_PTR(-ENOMEM);
+	if (!(inode_state_read_once(inode) & I_NEW))
+		return inode;
+
+	err = hfs_system_read_inode(inode);
+	if (err) {
+		iget_failed(inode);
+		return ERR_PTR(err);
+	}
+
+	unlock_new_inode(inode);
+	return inode;
+}
+
 /*
  * __hfs_iget()
  *
-- 
2.39.5

Re: [PATCH v2 1/2] hfs: create the internal B-tree inodes through the generic iget path

[Viacheslav Dubeyko]

On Mon, 2026-10-12 at 15:44 +0800, 1sh1ro wrote:
> hfs_btree_open() builds the extents and catalog B-tree inodes with a
> raw
> iget_locked() and never assigns them a file type, so both stay cached
> with
> i_mode == 0 after every mount.  A crafted HFS image whose catalog
> contains
> a record addressing one of the reserved B-tree CNIDs (3 or 4) makes
> hfs_lookup() -> hfs_iget() return that cached inode -
> hfs_test_inode()
> matches on i_ino alone - and the typeless inode then reaches the
> generic
> open path, where it trips VFS_BUG_ON_INODE() in may_open() with
> CONFIG_DEBUG_VFS=y:
> 
>   VFS_BUG_ON_INODE(!IS_ANON_FILE(inode)): inode:... fs:hfs mode:0
> opflags:0xc
>   ------------[ 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
> 
> Without CONFIG_DEBUG_VFS the assertion compiles away and the same
> image
> silently yields an openable typeless inode, so this is not merely a
> debug-config nuisance.
> 
> Fix it the way HFS+ handles its system inodes: add
> hfs_system_read_inode(),
> which reads the fork from the MDB (the counterpart of
> hfsplus_system_read_inode()), and hfs_system_iget(), which looks the
> inode
> up through the normal inode-cache path and can fail before the inode
> is
> published.  hfs_btree_open() now uses hfs_system_iget(), so the B-
> tree
> inodes carry a valid S_IFREG type like every other inode.  The
> extent-record validation is unchanged; it just runs after the inode
> has
> been created.
> 
> Found by syzkaller-style fuzzing of v7.2-rc7; still reproducible on
> current mainline.
> 
> Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
> Link:
> https://lore.kernel.org/linux-fsdevel/tencent_24D928451FEFA8A35482EFFA9132A7305307@qq.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: 1sh1ro <yangguikun19@qq.com>

Please, provide the real name.

> ---
>   fs/hfs/btree.c  |  25 +++++--------------------
>  fs/hfs/hfs_fs.h |   1 +
>  fs/hfs/inode.c  |  74 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 41b4e8f..7486707 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -155,44 +155,29 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
>  	tree->cnid = id;
>  	tree->keycmp = keycmp;
>  
> -	tree->inode = iget_locked(sb, id);
> -	if (!tree->inode)
> +	tree->inode = hfs_system_iget(sb, id);
> +	if (IS_ERR(tree->inode))
>  		goto free_tree;
> -	BUG_ON(!(inode_state_read_once(tree->inode) & I_NEW));
> -	{
> -	struct hfs_mdb *mdb = HFS_SB(sb)->mdb;
> -	HFS_I(tree->inode)->flags = 0;
> -	mutex_init(&HFS_I(tree->inode)->extents_lock);
> +
>  	switch (id) {
>  	case HFS_EXT_CNID:
> -		hfs_inode_read_fork(tree->inode, mdb->drXTExtRec,
> mdb->drXTFlSize,
> -				    mdb->drXTFlSize,
> be32_to_cpu(mdb->drXTClpSiz));
>  		if (HFS_I(tree->inode)->alloc_blocks >
>  					HFS_I(tree->inode)-
> >first_blocks) {
>  			pr_err("invalid btree extent records\n");

This check should be in hfs_system_read_inode() now.

> -			unlock_new_inode(tree->inode);
>  			goto free_inode;
>  		}
> -
> -		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
>  		break;
>  	case HFS_CAT_CNID:
> -		hfs_inode_read_fork(tree->inode, mdb->drCTExtRec,
> mdb->drCTFlSize,
> -				    mdb->drCTFlSize,
> be32_to_cpu(mdb->drCTClpSiz));
> -
>  		if (!HFS_I(tree->inode)->first_blocks) {
>  			pr_err("invalid btree extent records (0
> size)\n");

Ditto. This check should be in hfs_system_read_inode() now.

> -			unlock_new_inode(tree->inode);
>  			goto free_inode;
>  		}
> -
> -		tree->inode->i_mapping->a_ops = &hfs_btree_aops;
>  		break;
>  	default:
>  		BUG();
>  	}
> -	}
> -	unlock_new_inode(tree->inode);
> +
> +	tree->inode->i_mapping->a_ops = &hfs_btree_aops;
>  
>  	mapping = tree->inode->i_mapping;
>  	folio = filemap_grab_folio(mapping, 0);
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index e250f87..26505f7 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -219,6 +219,7 @@ extern void hfs_inode_read_fork(struct inode
> *inode, struct hfs_extent *ext,
>  				u32 clump_size);
>  extern struct inode *hfs_iget(struct super_block *sb, struct
> hfs_cat_key *key,
>  				hfs_cat_rec *rec);
> +extern struct inode *hfs_system_iget(struct super_block *sb, u32
> cnid);
>  extern void hfs_evict_inode(struct inode *inode);
>  extern void hfs_delete_inode(struct inode *inode);
>  
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 2aef3c3..4316d75 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -421,6 +421,80 @@ static int hfs_read_inode(struct inode *inode,
> void *data)
>  	return 0;
>  }
>  
> +/*
> + * hfs_system_read_inode

You need to place short abstract of the function:
hfs_system_read_inode() - <short explanation> or simply remove it
because there is no useful info here.

> + *
> + * Initialize one of the internal B-tree inodes.  These are
> described by the
> + * MDB rather than by catalog records, so they need their own read
> helper -
> + * the HFS counterpart of hfsplus_system_read_inode().

Could you please rework comment? It is not useful now.

> + */
> +static int hfs_system_read_inode(struct inode *inode)
> +{
> +	struct hfs_sb_info *hsb = HFS_SB(inode->i_sb);
> +	struct hfs_mdb *mdb = hsb->mdb;
> +
> +	HFS_I(inode)->flags = 0;
> +	HFS_I(inode)->rsrc_inode = NULL;
> +	mutex_init(&HFS_I(inode)->extents_lock);
> +	HFS_I(inode)->tz_secondswest = sys_tz.tz_minuteswest * 60;
> +
> +	inode->i_uid = hsb->s_uid;
> +	inode->i_gid = hsb->s_gid;
> +	set_nlink(inode, 1);
> +
> +	switch (inode->i_ino) {
> +	case HFS_EXT_CNID:
> +		hfs_inode_read_fork(inode, mdb->drXTExtRec, mdb-
> >drXTFlSize,
> +				    mdb->drXTFlSize,
> be32_to_cpu(mdb->drXTClpSiz));
> +		break;
> +	case HFS_CAT_CNID:
> +		hfs_inode_read_fork(inode, mdb->drCTExtRec, mdb-
> >drCTFlSize,
> +				    mdb->drCTFlSize,
> be32_to_cpu(mdb->drCTClpSiz));
> +		break;
> +	default:
> +		return -EIO;
> +	}
> +
> +	/*
> +	 * Assign a dummy file type, for may_open() requires that an
> inode has
> +	 * a valid file type.  These inodes are hashed by CNID like
> any other
> +	 * inode, so without a type a catalog record of a crafted
> image that
> +	 * aliases CNID 3 or 4 would reach the VFS with i_mode == 0.
> +	 */

Could it be short and clear comment?

> +	inode->i_mode = S_IFREG;
> +
> +	return 0;
> +}
> +
> +/*
> + * hfs_system_iget

Ditto.

> + *
> + * Look up (or create) one of the internal B-tree inodes. 
> hfs_btree_open()
> + * uses this instead of a raw iget_locked() so that the inode is
> completely
> + * initialized before it becomes visible in the inode cache, in the
> same way
> + * hfsplus_iget() handles the system CNIDs for HFS+.

Could you please rework the AI generated comment? Currently, it is not
useful.

Thanks,
Slava.

> + */
> +struct inode *hfs_system_iget(struct super_block *sb, u32 cnid)
> +{
> +	struct inode *inode;
> +	int err;
> +
> +	inode = iget_locked(sb, cnid);
> +	if (!inode)
> +		return ERR_PTR(-ENOMEM);
> +	if (!(inode_state_read_once(inode) & I_NEW))
> +		return inode;
> +
> +	err = hfs_system_read_inode(inode);
> +	if (err) {
> +		iget_failed(inode);
> +		return ERR_PTR(err);
> +	}
> +
> +	unlock_new_inode(inode);
> +	return inode;
> +}
> +
>  /*
>   * __hfs_iget()
>   *