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

[PATCH v5] 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 or that its size matches the thread name.

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 bounds the
record read to sizeof(hfsplus_cat_entry) and verifies that a thread
record's size exactly matches nodeName.length. Together, these checks
ensure an accepted thread name fits HFSPLUS_MAX_STRLEN. Reject non-thread
records before building the delete key from the validated thread name.

Share the thread-record-type helper between hfsplus_find_cat() and
hfsplus_delete_cat().

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

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 776ce36cf076..fe11c4b6dd99 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -204,7 +204,7 @@ int hfsplus_find_cat(struct super_block *sb, u32 cnid,
 		return err;
 
 	type = be16_to_cpu(tmp.type);
-	if (type != HFSPLUS_FOLDER_THREAD && type != HFSPLUS_FILE_THREAD) {
+	if (!is_hfs_thread_record_type(type)) {
 		pr_err("found bad thread record in catalog\n");
 		return -EIO;
 	}
@@ -350,23 +350,22 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, const struct qstr *str)
 		goto out;
 
 	if (!str) {
-		int len;
+		hfsplus_cat_entry entry = {0};
 
 		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);
+		type = be16_to_cpu(entry.type);
+		if (!is_hfs_thread_record_type(type)) {
+			pr_err("found bad thread record in catalog\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..7c8667d5a49c 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -521,6 +521,12 @@ 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_hfs_thread_record_type(u16 type)
+{
+	return type == HFSPLUS_FOLDER_THREAD || type == HFSPLUS_FILE_THREAD;
+}
+
 int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry);
 
 /*
-- 
2.54.0

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

[Viacheslav Dubeyko]

On Wed, 2026-07-08 at 18:02 -0700, Kyle Zeng wrote:
> 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 or that its size matches the thread name.
> 
> 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 bounds
> the
> record read to sizeof(hfsplus_cat_entry) and verifies that a thread
> record's size exactly matches nodeName.length. Together, these checks
> ensure an accepted thread name fits HFSPLUS_MAX_STRLEN. Reject non-
> thread
> records before building the delete key from the validated thread
> name.
> 
> Share the thread-record-type helper between hfsplus_find_cat() and
> hfsplus_delete_cat().
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Kyle Zeng <kylebot@openai.com>
> ---
>  fs/hfsplus/catalog.c    | 25 ++++++++++++-------------
>  fs/hfsplus/hfsplus_fs.h |  6 ++++++
>  2 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
> index 776ce36cf076..fe11c4b6dd99 100644
> --- a/fs/hfsplus/catalog.c
> +++ b/fs/hfsplus/catalog.c
> @@ -204,7 +204,7 @@ int hfsplus_find_cat(struct super_block *sb, u32
> cnid,
>  		return err;
>  
>  	type = be16_to_cpu(tmp.type);
> -	if (type != HFSPLUS_FOLDER_THREAD && type !=
> HFSPLUS_FILE_THREAD) {
> +	if (!is_hfs_thread_record_type(type)) {
>  		pr_err("found bad thread record in catalog\n");
>  		return -EIO;
>  	}
> @@ -350,23 +350,22 @@ int hfsplus_delete_cat(u32 cnid, struct inode
> *dir, const struct qstr *str)
>  		goto out;
>  
>  	if (!str) {
> -		int len;
> +		hfsplus_cat_entry entry = {0};
>  
>  		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);
> +		type = be16_to_cpu(entry.type);
> +		if (!is_hfs_thread_record_type(type)) {
> +			pr_err("found bad thread record in
> catalog\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..7c8667d5a49c 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -521,6 +521,12 @@ 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_hfs_thread_record_type(u16 type)
> +{
> +	return type == HFSPLUS_FOLDER_THREAD || type ==
> HFSPLUS_FILE_THREAD;
> +}
> +
>  int hfsplus_brec_read_cat(struct hfs_find_data *fd,
> hfsplus_cat_entry *entry);
>  
>  /*

OK. Let's minimize the fix only by these modifications. The patch looks
good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.