[PATCH] erofs: delimit inode_share cache key components

[PATCH] erofs: delimit inode_share cache key components

[ChengyuZhu6]

From: Chengyu Zhu <hudsonzhu@tencent.com>

The selected fingerprint xattr is treated as opaque and accepts any
positive length up to the filesystem block size.  Appending the domain ID
directly to such a value leaves no boundary between the two key components
if a different fingerprint length is used.

Change the key encoding as follows:

  fingerprint || domain_id  ->  domain_id || '\0' || fingerprint

Since a mount option string cannot contain NUL, this makes the in-memory
tuple unambiguous without constraining fingerprint contents or changing
the on-disk format.

Signed-off-by: Chengyu Zhu <hudsonzhu@tencent.com>
---
 fs/erofs/xattr.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index df7ea019526d..17c640aca5f7 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -621,6 +621,7 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 	struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
 	struct erofs_xattr_prefix_item *prefix;
 	const char *infix;
+	size_t domainlen;
 	int valuelen, base_index;
 
 	if (!test_opt(&sbi->opt, INODE_SHARE))
@@ -633,17 +634,19 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 	valuelen = erofs_getxattr(inode, base_index, infix, NULL, 0);
 	if (valuelen <= 0 || valuelen > (1 << sbi->blkszbits))
 		return -EFSCORRUPTED;
-	fp->size = valuelen + (domain_id ? strlen(domain_id) : 0);
+	domain_id = domain_id ?: "";
+	domainlen = strlen(domain_id);
+	fp->size = domainlen + 1 + valuelen;
 	fp->opaque = kmalloc(fp->size, GFP_KERNEL);
 	if (!fp->opaque)
 		return -ENOMEM;
+	memcpy(fp->opaque, domain_id, domainlen + 1);
 	if (valuelen != erofs_getxattr(inode, base_index, infix,
-				       fp->opaque, valuelen)) {
+				       fp->opaque + domainlen + 1, valuelen)) {
 		kfree(fp->opaque);
 		fp->opaque = NULL;
 		return -EFSCORRUPTED;
 	}
-	memcpy(fp->opaque + valuelen, domain_id, fp->size - valuelen);
 	return 0;
 }
 #endif
-- 
2.51.0

Re: [PATCH] erofs: delimit inode_share cache key components

[Gao Xiang]

Hi Chengyu,

On Mon, Sep 07, 2026 at 03:24:40PM +0800, ChengyuZhu6 wrote:
> From: Chengyu Zhu <hudsonzhu@tencent.com>
> 
> The selected fingerprint xattr is treated as opaque and accepts any
> positive length up to the filesystem block size.  Appending the domain ID
> directly to such a value leaves no boundary between the two key components
> if a different fingerprint length is used.
> 
> Change the key encoding as follows:
> 
>   fingerprint || domain_id  ->  domain_id || '\0' || fingerprint
> 
> Since a mount option string cannot contain NUL, this makes the in-memory
> tuple unambiguous without constraining fingerprint contents or changing
> the on-disk format.
> 
> Signed-off-by: Chengyu Zhu <hudsonzhu@tencent.com>

Subject: erofs: delimit inode_share key components

Previously, inode_share keys are encoded as follows:
    fingerprint || domain_id

It would be better to have a separator between fingerprint and domain_id
so that fingerprint won't be parsed as part of a domain_id.

Change the key encoding as follows:
    domain_id || '\0' || fingerprint

Since domain_id is a NIL-terminated string, this makes the in-memory
key indices unambiguous.

> ---
>  fs/erofs/xattr.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index df7ea019526d..17c640aca5f7 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -621,6 +621,7 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
>  	struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
>  	struct erofs_xattr_prefix_item *prefix;
>  	const char *infix;
> +	size_t domainlen;
>  	int valuelen, base_index;
>  
>  	if (!test_opt(&sbi->opt, INODE_SHARE))
> @@ -633,17 +634,19 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
>  	valuelen = erofs_getxattr(inode, base_index, infix, NULL, 0);
>  	if (valuelen <= 0 || valuelen > (1 << sbi->blkszbits))
>  		return -EFSCORRUPTED;
> -	fp->size = valuelen + (domain_id ? strlen(domain_id) : 0);
> +	domain_id = domain_id ?: "";
> +	domainlen = strlen(domain_id);
> +	fp->size = domainlen + 1 + valuelen;
>  	fp->opaque = kmalloc(fp->size, GFP_KERNEL);
>  	if (!fp->opaque)
>  		return -ENOMEM;
> +	memcpy(fp->opaque, domain_id, domainlen + 1);
>  	if (valuelen != erofs_getxattr(inode, base_index, infix,
> -				       fp->opaque, valuelen)) {
> +				       fp->opaque + domainlen + 1, valuelen)) {
>  		kfree(fp->opaque);
>  		fp->opaque = NULL;
>  		return -EFSCORRUPTED;
>  	}
> -	memcpy(fp->opaque + valuelen, domain_id, fp->size - valuelen);
>  	return 0;

Can you apply the following diff? domain_id cannot be NULL so it was a
redundant check:

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index 17c640aca5f7..57cfb7520782 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -620,9 +620,8 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 {
 	struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
 	struct erofs_xattr_prefix_item *prefix;
+	int domainlen, valuelen, base_index;
 	const char *infix;
-	size_t domainlen;
-	int valuelen, base_index;
 
 	if (!test_opt(&sbi->opt, INODE_SHARE))
 		return -EOPNOTSUPP;
@@ -634,7 +633,6 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 	valuelen = erofs_getxattr(inode, base_index, infix, NULL, 0);
 	if (valuelen <= 0 || valuelen > (1 << sbi->blkszbits))
 		return -EFSCORRUPTED;
-	domain_id = domain_id ?: "";
 	domainlen = strlen(domain_id);
 	fp->size = domainlen + 1 + valuelen;
 	fp->opaque = kmalloc(fp->size, GFP_KERNEL);


>  }
>  #endif
> -- 
> 2.51.0

[PATCH v2] erofs: delimit inode_share cache key components

[ChengyuZhu6]

From: Chengyu Zhu <hudsonzhu@tencent.com>

Previously, inode_share keys were encoded as follows:

  fingerprint || domain_id

It would be better to have a separator between the fingerprint and domain
ID so that the fingerprint won't be parsed as part of a domain ID.

Change the key encoding as follows:

  domain_id || '\0' || fingerprint

Since domain_id is a NUL-terminated string, this makes the in-memory key
indices unambiguous.

Signed-off-by: Chengyu Zhu <hudsonzhu@tencent.com>
---
Changes since v1:
- Use int for domainlen and drop the redundant NULL domain_id handling.

 fs/erofs/xattr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
index df7ea019526d..57cfb7520782 100644
--- a/fs/erofs/xattr.c
+++ b/fs/erofs/xattr.c
@@ -620,7 +620,7 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 {
 	struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb);
 	struct erofs_xattr_prefix_item *prefix;
+	int domainlen, valuelen, base_index;
 	const char *infix;
-	int valuelen, base_index;
 
 	if (!test_opt(&sbi->opt, INODE_SHARE))
@@ -633,17 +633,18 @@ int erofs_xattr_fill_inode_fingerprint(struct erofs_inode_fingerprint *fp,
 	valuelen = erofs_getxattr(inode, base_index, infix, NULL, 0);
 	if (valuelen <= 0 || valuelen > (1 << sbi->blkszbits))
 		return -EFSCORRUPTED;
-	fp->size = valuelen + (domain_id ? strlen(domain_id) : 0);
+	domainlen = strlen(domain_id);
+	fp->size = domainlen + 1 + valuelen;
 	fp->opaque = kmalloc(fp->size, GFP_KERNEL);
 	if (!fp->opaque)
 		return -ENOMEM;
+	memcpy(fp->opaque, domain_id, domainlen + 1);
 	if (valuelen != erofs_getxattr(inode, base_index, infix,
-				       fp->opaque, valuelen)) {
+				       fp->opaque + domainlen + 1, valuelen)) {
 		kfree(fp->opaque);
 		fp->opaque = NULL;
 		return -EFSCORRUPTED;
 	}
-	memcpy(fp->opaque + valuelen, domain_id, fp->size - valuelen);
 	return 0;
 }
 #endif
-- 
2.51.0

Re: [PATCH v2] erofs: delimit inode_share cache key components

[Gao Xiang]

On Mon, Sep 07, 2026 at 04:33:19PM +0800, ChengyuZhu6 wrote:
> From: Chengyu Zhu <hudsonzhu@tencent.com>
> 
> Previously, inode_share keys were encoded as follows:
> 
>   fingerprint || domain_id
> 
> It would be better to have a separator between the fingerprint and domain
> ID so that the fingerprint won't be parsed as part of a domain ID.
> 
> Change the key encoding as follows:
> 
>   domain_id || '\0' || fingerprint
> 
> Since domain_id is a NUL-terminated string, this makes the in-memory key
> indices unambiguous.
> 
> Signed-off-by: Chengyu Zhu <hudsonzhu@tencent.com>

Reviewed-by: Gao Xiang <xiang@kernel.org>

Thanks,
Gao Xiang