[PATCH 0/2] hfs&hfsplus: abort for unexpected name length

[PATCH 0/2] hfs&hfsplus: abort for unexpected name length

[Yangtao Li]

We need to prevent to create unexpected files, which name length over
NAMELEN_MAX.

Yangtao Li (2):
  hfs: abort hfs_lookup if name is too long
  hfsplus: abort hfsplus_lookup if name is too long

 fs/hfs/dir.c     | 3 +++
 fs/hfsplus/dir.c | 3 +++
 2 files changed, 6 insertions(+)

-- 
2.48.1

[PATCH 1/2] hfs: abort hfs_lookup if name is too long

[Yangtao Li]

Long file names for hfs is 31 characters.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/hfs/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
index 86a6b317b474..30f6194da939 100644
--- a/fs/hfs/dir.c
+++ b/fs/hfs/dir.c
@@ -25,6 +25,9 @@ static struct dentry *hfs_lookup(struct inode *dir, struct dentry *dentry,
 	struct inode *inode = NULL;
 	int res;
 
+	if (dentry->d_name.len > HFS_NAMELEN)
+		return ERR_PTR(-ENAMETOOLONG);
+
 	res = hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
 	if (res)
 		return ERR_PTR(res);
-- 
2.48.1

Re: [PATCH 1/2] hfs: abort hfs_lookup if name is too long

[Viacheslav Dubeyko]

On Wed, 2025-08-06 at 11:11 -0600, Yangtao Li wrote:
> Long file names for hfs is 31 characters.
> 

Could this max name length affects the xfstests in the case if we
finally restricts the creation of files/folders with longer names?

> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfs/dir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
> index 86a6b317b474..30f6194da939 100644
> --- a/fs/hfs/dir.c
> +++ b/fs/hfs/dir.c
> @@ -25,6 +25,9 @@ static struct dentry *hfs_lookup(struct inode *dir,
> struct dentry *dentry,
>  	struct inode *inode = NULL;
>  	int res;
>  
> +	if (dentry->d_name.len > HFS_NAMELEN)
> +		return ERR_PTR(-ENAMETOOLONG);
> +

I think it makes sense to follow the HFS+ logic. We need to rework
hfs_cat_build_key() [1, 2] and hfs_asc2mac() [3]. It already operates
by -ENAMETOOLONG [4] but it is not we would like to have.

Thanks,
Slava.

>  	res = hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
>  	if (res)
>  		return ERR_PTR(res);

[1] https://elixir.bootlin.com/linux/v6.16/source/fs/hfs/dir.c#L31
[2] https://elixir.bootlin.com/linux/v6.16/source/fs/hfs/catalog.c#L28
[3] https://elixir.bootlin.com/linux/v6.16/source/fs/hfs/trans.c#L97
[4] https://elixir.bootlin.com/linux/v6.16/source/fs/hfs/trans.c#L125

[PATCH 2/2] hfsplus: abort hfsplus_lookup if name is too long

[Yangtao Li]

Long file names for hfs is 255 characters.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/hfsplus/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 876bbb80fb4d..d8fb401e7fdc 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -38,6 +38,9 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
 	u32 cnid, linkid = 0;
 	u16 type;
 
+	if (dentry->d_name.len > HFSPLUS_MAX_STRLEN)
+		return ERR_PTR(-ENAMETOOLONG);
+
 	sb = dir->i_sb;
 
 	dentry->d_fsdata = NULL;
-- 
2.48.1

Re: [PATCH 2/2] hfsplus: abort hfsplus_lookup if name is too long

[Viacheslav Dubeyko]

On Wed, 2025-08-06 at 11:11 -0600, Yangtao Li wrote:
> Long file names for hfs is 255 characters.

You already mentioned in another patch that HFS has limitation in 31
symbols. I think this patch requires more explanation why you've
selected hfsplus_lookup().

> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>  fs/hfsplus/dir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index 876bbb80fb4d..d8fb401e7fdc 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -38,6 +38,9 @@ static struct dentry *hfsplus_lookup(struct inode
> *dir, struct dentry *dentry,
>  	u32 cnid, linkid = 0;
>  	u16 type;
>  
> +	if (dentry->d_name.len > HFSPLUS_MAX_STRLEN)
> +		return ERR_PTR(-ENAMETOOLONG);
> +

Are you sure that we really need to abort the hfsplus_lookup()? We
already have the logic that checks the name length. We call
hfsplus_cat_build_key() [1], then hfsplus_asc2uni() [2]. And
hfsplus_asc2uni() checks the maximum name length and it returns -
ENAMETOOLONG [3].

Thanks,
Slava.

>  	sb = dir->i_sb;
>  
>  	dentry->d_fsdata = NULL;

[1] https://elixir.bootlin.com/linux/v6.16/source/fs/hfsplus/dir.c#L47
[2]
https://elixir.bootlin.com/linux/v6.16/source/fs/hfsplus/catalog.c#L49
[3]
https://elixir.bootlin.com/linux/v6.16/source/fs/hfsplus/unicode.c#L375