[PATCH v2] hfsplus: validate thread record before delete key rebuild

[PATCH v2] hfsplus: validate thread record before delete key rebuild

[Kyle Zeng]

hfsplus_delete_cat() is called with str == NULL when the last open
reference to an unlinked HFS+ hardlink backing inode is closed. In that
case, the function finds the catalog thread by CNID and rebuilds the
catalog key from thread.nodeName.

That reconstruction path reads thread.nodeName.length directly from the
catalog B-tree into fd.search_key and then copies length * 2 bytes into
fd.search_key->cat.name.unicode. It does not first check that the found
record is a thread record, that the record size matches the thread name
length, or that nodeName.length fits in the fixed HFS+ catalog key
buffer.

A corrupted image can therefore provide an oversized thread name length
and make hfs_bnode_read() write past the catalog search-key allocation.

Read the CNID record through hfsplus_brec_read_cat(), which performs the
hfs_brec_find() lookup via hfs_brec_read() before validating the catalog
record. Reject non-thread records and explicitly bound nodeName.length
before building the delete key from the validated thread name.

Factor the existing thread-type and name-length checks into shared
helpers so hfsplus_find_cat() and hfsplus_delete_cat() use the same
validation.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 fs/hfsplus/catalog.c    | 38 ++++++++++++++++++++++----------------
 fs/hfsplus/hfsplus_fs.h | 11 +++++++++++
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 776ce36cf076..c57c3bbb8143 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -196,20 +196,20 @@ int hfsplus_find_cat(struct super_block *sb, u32 cnid,
 {
 	hfsplus_cat_entry tmp = {0};
 	int err;
-	u16 type;
+	u16 entry_type;
 
 	hfsplus_cat_build_key_with_cnid(sb, fd->search_key, cnid);
 	err = hfsplus_brec_read_cat(fd, &tmp);
 	if (err)
 		return err;
 
-	type = be16_to_cpu(tmp.type);
-	if (type != HFSPLUS_FOLDER_THREAD && type != HFSPLUS_FILE_THREAD) {
+	entry_type = be16_to_cpu(tmp.type);
+	if (!is_hfsplus_thread_type(entry_type)) {
 		pr_err("found bad thread record in catalog\n");
 		return -EIO;
 	}
 
-	if (be16_to_cpu(tmp.thread.nodeName.length) > 255) {
+	if (!is_hfsplus_name_length_valid(tmp.thread.nodeName.length)) {
 		pr_err("catalog name length corrupted\n");
 		return -EIO;
 	}
@@ -350,23 +350,29 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
 		goto out;
 
 	if (!str) {
-		int len;
+		hfsplus_cat_entry entry = {0};
+		u16 entry_type;
 
 		hfsplus_cat_build_key_with_cnid(sb, fd.search_key, cnid);
-		err = hfs_brec_find(&fd, hfs_find_rec_by_key);
+		err = hfsplus_brec_read_cat(&fd, &entry);
 		if (err)
 			goto out;
 
-		off = fd.entryoffset +
-			offsetof(struct hfsplus_cat_thread, nodeName);
-		fd.search_key->cat.parent = cpu_to_be32(dir->i_ino);
-		hfs_bnode_read(fd.bnode,
-			&fd.search_key->cat.name.length, off, 2);
-		len = be16_to_cpu(fd.search_key->cat.name.length) * 2;
-		hfs_bnode_read(fd.bnode,
-			&fd.search_key->cat.name.unicode,
-			off + 2, len);
-		fd.search_key->key_len = cpu_to_be16(6 + len);
+		entry_type = be16_to_cpu(entry.type);
+		if (!is_hfsplus_thread_type(entry_type)) {
+			pr_err("found bad thread record in catalog\n");
+			err = -EIO;
+			goto out;
+		}
+
+		if (!is_hfsplus_name_length_valid(entry.thread.nodeName.length)) {
+			pr_err("catalog name length corrupted\n");
+			err = -EIO;
+			goto out;
+		}
+
+		hfsplus_cat_build_key_uni(fd.search_key, dir->i_ino,
+					  &entry.thread.nodeName);
 	} else {
 		err = hfsplus_cat_build_key(sb, fd.search_key, dir->i_ino, str);
 		if (unlikely(err))
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index ec04b82ad927..b954b45a1003 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -521,6 +521,17 @@ static inline u32 hfsplus_cat_thread_size(const struct hfsplus_cat_thread *threa
 	       be16_to_cpu(thread->nodeName.length) * sizeof(hfsplus_unichr);
 }
 
+static inline bool is_hfsplus_thread_type(u16 entry_type)
+{
+	return entry_type == HFSPLUS_FOLDER_THREAD ||
+	       entry_type == HFSPLUS_FILE_THREAD;
+}
+
+static inline bool is_hfsplus_name_length_valid(__be16 name_length)
+{
+	return be16_to_cpu(name_length) <= HFSPLUS_MAX_STRLEN;
+}
+
 int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
 
 /*
-- 
2.54.0