[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

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

[Viacheslav Dubeyko]

On Wed, 2026-07-01 at 00:35 -0600, 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, 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;

I think that this change is not good. The name type is good enough and
it is shorter one. It is not critical remark but this change doesn't
make sense to me.

>  
>  	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);
>  
>  /*

Mostly, patch looks good. Let me run xfstests for the patch. I'll share
my final review ASAP.

Thanks,
Slava.

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

[Viacheslav Dubeyko]

Hi Kyle,

The xfestests haven't revealed any new issues. So, patch looks good
but, please, check my remarks below. I see some logical issues in the
patch that needs to be reworked from my point of view.

On Thu, 2026-07-02 at 17:13 -0700, Viacheslav Dubeyko wrote:
> On Wed, 2026-07-01 at 00:35 -0600, 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, 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;
> 
> I think that this change is not good. The name type is good enough
> and
> it is shorter one. It is not critical remark but this change doesn't
> make sense to me.
> 
> >  
> >  	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)

First of all, HFS and HFS+ both use the thread records. Also,
is_hfsplus_thread_type() sounds slightly confusing. So, I suggest to
use is_hfs_thread_record_type() name. First of all, we are checking the
thread record type. And, secondly, we can re-use this function in the
future. The difference is that HFS uses HFS_CDR_THD/HFS_CDR_FTH
constants for thread records.

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

After some consideration, I cannot accept
is_hfsplus_name_length_valid() as a completely correct check. Because,
even if name_length is smaller than HFSPLUS_MAX_STRLEN, then it doesn't
mean that we haven't a record corruption. Because, the name_length can
be smaller one that HFSPLUS_MAX_STRLEN but still bigger one that the
real length of the name or the length of record itself. Also, xattr
name cannot be bigger than HFSPLUS_ATTR_MAX_STRLEN (127) but it still
HFS+ name. So, I think we need to rework the logic of this function to
check the name length more carefully. And I believe that function name
needs to be reworked too slightly.

Thanks,
Slava.

> > +}
> > +
> >  int hfsplus_brec_read_cat(struct hfs_find_data *fd,
> > hfsplus_cat_entry *entry);
> >  
> >  /*
> 
> 

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

[Kyle Zeng]

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

Hi Slava,

A new patch following your suggestion is attached. Please let me know if
you prefer to receive the new patch in a separate thread.

Thanks,
Kyle

[-- Attachment #2: 0001-hfsplus-validate-thread-record-before-delete-key-rebuild.patch --]
[-- Type: text/plain, Size: 4659 bytes --]

From 25375348bf897058e2c47806bc9cf1d3dd6b2647 Mon Sep 17 00:00:00 2001
From: Kyle Zeng <kylebot@openai.com>
Date: Tue, 7 Jul 2026 18:37:32 +0000
Subject: [PATCH v3] hfsplus: validate thread record before delete key rebuild

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 before building the delete key from
the validated thread name.

Move the HFS+ catalog thread name bound into hfsplus_brec_read_cat() so
it is checked together with the record-size consistency validation, and
factor the thread-record-type check into a shared HFS/HFS+ helper.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 fs/hfsplus/bfind.c         |  5 +++++
 fs/hfsplus/catalog.c       | 30 ++++++++++++------------------
 include/linux/hfs_common.h |  5 +++++
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/fs/hfsplus/bfind.c b/fs/hfsplus/bfind.c
index 9a55fa6d5294..417c3014f4c7 100644
--- a/fs/hfsplus/bfind.c
+++ b/fs/hfsplus/bfind.c
@@ -322,6 +322,11 @@ int hfsplus_brec_read_cat(struct hfs_find_data *fd, hfsplus_cat_entry *entry)
 			pr_err("thread record too short (got %u)\n", fd->entrylength);
 			return -EIO;
 		}
+		if (be16_to_cpu(entry->thread.nodeName.length) >
+		    HFSPLUS_MAX_STRLEN) {
+			pr_err("catalog name length corrupted\n");
+			return -EIO;
+		}
 		expected_size = hfsplus_cat_thread_size(&entry->thread);
 		break;
 	default:
diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index 776ce36cf076..1a4b0beae135 100644
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -204,16 +204,11 @@ 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;
 	}
 
-	if (be16_to_cpu(tmp.thread.nodeName.length) > 255) {
-		pr_err("catalog name length corrupted\n");
-		return -EIO;
-	}
-
 	hfsplus_cat_build_key_uni(fd->search_key,
 		be32_to_cpu(tmp.thread.parentID),
 		&tmp.thread.nodeName);
@@ -350,23 +345,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/include/linux/hfs_common.h b/include/linux/hfs_common.h
index 45fb4c9ff9f5..c4862166efc9 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -92,6 +92,11 @@
 #define HFS_CDR_THD		0x03	/* folder (directory) thread */
 #define HFS_CDR_FTH		0x04	/* file thread */
 
+static inline bool is_hfs_thread_record_type(u16 type)
+{
+	return type == HFS_CDR_THD || type == HFS_CDR_FTH;
+}
+
 /* legal values for hfs_ext_key.FkType and hfs_file.fork */
 #define HFS_FK_DATA		0x00
 #define HFS_FK_RSRC		0xFF
-- 
2.54.0

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

[Viacheslav Dubeyko]

On Tue, 2026-07-07 at 11:48 -0700, Kyle Zeng wrote:
> Hi Slava,
> 
> A new patch following your suggestion is attached. Please let me know
> if
> you prefer to receive the new patch in a separate thread.
> 

Could you please send the next version of formal patch?

Thanks,
Slava.