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