Re: [syzbot] [hfs?] kernel BUG in hfs_write_inode

[George Anthony Vernon <contact@gvernon.com>]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]

On Thu, Oct 02, 2025 at 05:18:03PM -0700, syzbot wrote:
> Hello,
> 
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> 
> Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
> Tested-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
> 
> Tested on:
> 
> commit:         e5f0a698 Linux 6.17
> git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.17
> console output: https://syzkaller.appspot.com/x/log.txt?x=160acee2580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=f5b21423ca3f0a96
> dashboard link: https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
> compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=11089334580000
> 
> Note: testing is done by a robot and is best-effort only.

#syz test

[-- Attachment #2: 0001-hfs-Validate-CNIDs-in-hfs_read_inode.patch --]
[-- Type: text/plain, Size: 2490 bytes --]

From 5ff1f6bf582a643bce73f6a1c431bfe540f76b8a Mon Sep 17 00:00:00 2001
From: George Anthony Vernon <contact@gvernon.com>
Date: Fri, 3 Oct 2025 01:41:24 +0100
Subject: [PATCH] hfs: Validate CNIDs in hfs_read_inode

hfs_read_inode previously did not validate CNIDs read from disk, thereby
allowing bad inodes to be constructed and placed on the dirty list,
eventually hitting a bug on writeback.

Validate reserved CNIDs according to Apple technical note TN1150.

This issue was discussed on LKML previously:
https://lore.kernel.org/all/427fcb57-8424-4e52-9f21-7041b2c4ae5b@
I-love.SAKURA.ne.jp/T/

Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
Signed-off-by: George Anthony Vernon <contact@gvernon.com>
---
 fs/hfs/inode.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 9cd449913dc8..da6a6b32d8c2 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -321,6 +321,34 @@ static int hfs_test_inode(struct inode *inode, void *data)
 	}
 }
 
+/*
+ * is_valid_cnid
+ *
+ * Validate the CNID of a catalog record read from disk
+ */
+static bool is_valid_cnid(unsigned long cnid, s8 type)
+{
+	if (likely(cnid >= HFS_FIRSTUSER_CNID))
+		return true;
+
+	switch (cnid) {
+	case HFS_POR_CNID:
+		return type == HFS_CDR_DIR;
+	case HFS_ROOT_CNID:
+		return type == HFS_CDR_DIR;
+	case HFS_EXT_CNID:
+		return type == HFS_CDR_FIL;
+	case HFS_CAT_CNID:
+		return type == HFS_CDR_FIL;
+	case HFS_BAD_CNID:
+		return type == HFS_CDR_FIL;
+	case HFS_EXCH_CNID:
+		return type == HFS_CDR_FIL;
+	default:
+		return false;
+	}
+}
+
 /*
  * hfs_read_inode
  */
@@ -359,6 +387,11 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		}
 
 		inode->i_ino = be32_to_cpu(rec->file.FlNum);
+		if (!is_valid_cnid(inode->i_ino, HFS_CDR_FIL)) {
+			pr_warn("rejected cnid %lu\n", inode->i_ino);
+			make_bad_inode(inode);
+			break;
+		}
 		inode->i_mode = S_IRUGO | S_IXUGO;
 		if (!(rec->file.Flags & HFS_FIL_LOCK))
 			inode->i_mode |= S_IWUGO;
@@ -372,6 +405,11 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		break;
 	case HFS_CDR_DIR:
 		inode->i_ino = be32_to_cpu(rec->dir.DirID);
+		if (!is_valid_cnid(inode->i_ino, HFS_CDR_DIR)) {
+			pr_warn("rejected cnid %lu\n", inode->i_ino);
+			make_bad_inode(inode);
+			break;
+		}
 		inode->i_size = be16_to_cpu(rec->dir.Val) + 2;
 		HFS_I(inode)->fs_blocks = 0;
 		inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask);
-- 
2.50.1