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

[Gao Xiang <xiang@kernel.org>]

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