[PATCH] erofs: encode file handle with the internal helpers

[PATCH] erofs: encode file handle with the internal helpers

[Hongbo Li]

In erofs, the inode number has the location information of
files. The default encode_fh uses the ino32, this will lack
of some information when the file is too big. So we need
the internal helpers to encode filehandle.

Since i_generation in EROFS is not used, here we only encode
the nid into file handle when it is exported. So it is easy
to parse the dentry from file handle.

It is easy to reproduce test:
  1. prepare an erofs image with nid bigger than UINT_MAX
  2. mount -t erofs foo.img /mnt/erofs
  3. set exportfs with configuration: /mnt/erofs *(rw,sync,
     no_root_squash)
  4. mount -t nfs $IP:/mnt/erofs /mnt/nfs
  5. md5sum /mnt/nfs/foo # foo is the file which nid bigger
     than UINT_MAX.
For overlayfs case, the under filesystem's file handle is
encoded in ovl_fb.fid, it is same as NFS's case.

Fixes: 3e917cc305c6 ("erofs: make filesystem exportable")
Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 fs/erofs/super.c | 51 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index cadec6b1b554..8f787c47e04d 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -511,24 +511,59 @@ static int erofs_fc_parse_param(struct fs_context *fc,
 	return 0;
 }
 
-static struct inode *erofs_nfs_get_inode(struct super_block *sb,
-					 u64 ino, u32 generation)
+static int erofs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
+			   struct inode *parent)
 {
-	return erofs_iget(sb, ino);
+	int len = parent ? 4 : 2;
+	erofs_nid_t nid;
+
+	if (*max_len < len) {
+		*max_len = len;
+		return FILEID_INVALID;
+	}
+
+	nid = EROFS_I(inode)->nid;
+	fh[0] = (u32)(nid >> 32);
+	fh[1] = (u32)(nid & 0xffffffff);
+
+	if (parent) {
+		nid = EROFS_I(parent)->nid;
+
+		fh[2] = (u32)(nid >> 32);
+		fh[3] = (u32)(nid & 0xffffffff);
+	}
+
+	*max_len = len;
+	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
 }
 
 static struct dentry *erofs_fh_to_dentry(struct super_block *sb,
 		struct fid *fid, int fh_len, int fh_type)
 {
-	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
-				    erofs_nfs_get_inode);
+	erofs_nid_t nid;
+
+	if ((fh_type != FILEID_INO64_GEN &&
+	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 2)
+		return NULL;
+
+	nid = (u64) fid->raw[0] << 32;
+	nid |= (u64) fid->raw[1];
+
+	return d_obtain_alias(erofs_iget(sb, nid));
 }
 
 static struct dentry *erofs_fh_to_parent(struct super_block *sb,
 		struct fid *fid, int fh_len, int fh_type)
 {
-	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
-				    erofs_nfs_get_inode);
+	erofs_nid_t nid;
+
+	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 4)
+		return NULL;
+
+	nid = (u64) fid->raw[2] << 32;
+	nid |= (u64) fid->raw[3];
+
+	return d_obtain_alias(erofs_iget(sb, nid));
 }
 
 static struct dentry *erofs_get_parent(struct dentry *child)
@@ -544,7 +579,7 @@ static struct dentry *erofs_get_parent(struct dentry *child)
 }
 
 static const struct export_operations erofs_export_ops = {
-	.encode_fh = generic_encode_ino32_fh,
+	.encode_fh = erofs_encode_fh,
 	.fh_to_dentry = erofs_fh_to_dentry,
 	.fh_to_parent = erofs_fh_to_parent,
 	.get_parent = erofs_get_parent,
-- 
2.22.0

Re: [PATCH] erofs: encode file handle with the internal helpers

[Gao Xiang]

Hi Hongbo,

I think the subject can be updated as:
`erofs-utils: fix file handle encoding for 64-bit NIDs`

On Tue, Apr 29, 2025 at 01:11:39AM +0000, Hongbo Li wrote:
> In erofs, the inode number has the location information of
> files. The default encode_fh uses the ino32, this will lack
> of some information when the file is too big. So we need
> the internal helpers to encode filehandle.
> 
> Since i_generation in EROFS is not used, here we only encode
> the nid into file handle when it is exported. So it is easy
> to parse the dentry from file handle.

If `FILEID_INO64_GEN_PARENT` is used, I don't think
the generation number should be emitted, as documented as:

`
/*
 * 64 bit inode number, 32 bit generation number.
 */
FILEID_INO64_GEN = 0x81,

/*
 * 64 bit inode number, 32 bit generation number,
 * 64 bit parent inode number, 32 bit parent generation.
 */
FILEID_INO64_GEN_PARENT = 0x82,
` 

Even the generation number is 0 but we might use
i_generation for some remote update use cases
in the future.

> 
> It is easy to reproduce test:
>   1. prepare an erofs image with nid bigger than UINT_MAX
>   2. mount -t erofs foo.img /mnt/erofs
>   3. set exportfs with configuration: /mnt/erofs *(rw,sync,
>      no_root_squash)
>   4. mount -t nfs $IP:/mnt/erofs /mnt/nfs
>   5. md5sum /mnt/nfs/foo # foo is the file which nid bigger
>      than UINT_MAX.
> For overlayfs case, the under filesystem's file handle is
> encoded in ovl_fb.fid, it is same as NFS's case.

Can we have a way to add a testcase for the overlayfs case:
since it's somewhat complex to write a testcase with nfs
above.

> 
> Fixes: 3e917cc305c6 ("erofs: make filesystem exportable")
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  fs/erofs/super.c | 51 ++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 43 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index cadec6b1b554..8f787c47e04d 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -511,24 +511,59 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>  	return 0;
>  }
>  
> -static struct inode *erofs_nfs_get_inode(struct super_block *sb,
> -					 u64 ino, u32 generation)
> +static int erofs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> +			   struct inode *parent)
>  {
> -	return erofs_iget(sb, ino);
> +	int len = parent ? 4 : 2;
> +	erofs_nid_t nid;
> +
> +	if (*max_len < len) {
> +		*max_len = len;
> +		return FILEID_INVALID;
> +	}
> +
> +	nid = EROFS_I(inode)->nid;
> +	fh[0] = (u32)(nid >> 32);
> +	fh[1] = (u32)(nid & 0xffffffff);
> +
> +	if (parent) {
> +		nid = EROFS_I(parent)->nid;
> +
> +		fh[2] = (u32)(nid >> 32);
> +		fh[3] = (u32)(nid & 0xffffffff);
> +	}
> +
> +	*max_len = len;
> +	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>  }
>  
>  static struct dentry *erofs_fh_to_dentry(struct super_block *sb,
>  		struct fid *fid, int fh_len, int fh_type)
>  {
> -	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
> -				    erofs_nfs_get_inode);
> +	erofs_nid_t nid;
> +
> +	if ((fh_type != FILEID_INO64_GEN &&
> +	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 2)
> +		return NULL;
> +
> +	nid = (u64) fid->raw[0] << 32;
> +	nid |= (u64) fid->raw[1];
> +

Redundant new line.

> +	return d_obtain_alias(erofs_iget(sb, nid));
>  }
>  
>  static struct dentry *erofs_fh_to_parent(struct super_block *sb,
>  		struct fid *fid, int fh_len, int fh_type)
>  {
> -	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
> -				    erofs_nfs_get_inode);
> +	erofs_nid_t nid;
> +
> +	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 4)
> +		return NULL;
> +
> +	nid = (u64) fid->raw[2] << 32;
> +	nid |= (u64) fid->raw[3];
> +

Same here.

Thanks,
Gao Xiang

Re: [PATCH] erofs: encode file handle with the internal helpers

[Hongbo Li]

On 2025/4/29 11:31, Gao Xiang wrote:
> Hi Hongbo,
> 
> I think the subject can be updated as:
> `erofs-utils: fix file handle encoding for 64-bit NIDs`
> 
> On Tue, Apr 29, 2025 at 01:11:39AM +0000, Hongbo Li wrote:
>> In erofs, the inode number has the location information of
>> files. The default encode_fh uses the ino32, this will lack
>> of some information when the file is too big. So we need
>> the internal helpers to encode filehandle.
>>
>> Since i_generation in EROFS is not used, here we only encode
>> the nid into file handle when it is exported. So it is easy
>> to parse the dentry from file handle.
> 
> If `FILEID_INO64_GEN_PARENT` is used, I don't think
> the generation number should be emitted, as documented as:
> 
Ok, thanks for reviewing. I will change this in later version.
> `
> /*
>   * 64 bit inode number, 32 bit generation number.
>   */
> FILEID_INO64_GEN = 0x81,
> 
> /*
>   * 64 bit inode number, 32 bit generation number,
>   * 64 bit parent inode number, 32 bit parent generation.
>   */
> FILEID_INO64_GEN_PARENT = 0x82,
> `
> 
> Even the generation number is 0 but we might use
> i_generation for some remote update use cases
> in the future.
> 
>>
>> It is easy to reproduce test:
>>    1. prepare an erofs image with nid bigger than UINT_MAX
>>    2. mount -t erofs foo.img /mnt/erofs
>>    3. set exportfs with configuration: /mnt/erofs *(rw,sync,
>>       no_root_squash)
>>    4. mount -t nfs $IP:/mnt/erofs /mnt/nfs
>>    5. md5sum /mnt/nfs/foo # foo is the file which nid bigger
>>       than UINT_MAX.
>> For overlayfs case, the under filesystem's file handle is
>> encoded in ovl_fb.fid, it is same as NFS's case.
> 
> Can we have a way to add a testcase for the overlayfs case:
> since it's somewhat complex to write a testcase with nfs
> above.
> 
Yeah, I have a testcase can test this:

mount -t erofs foo.img /mnt/erofs
mount -t overlay overlay -o 
lowerdir=/mnt/erofs,upperdir=/mnt/upper,workdir=/mnt/work merged
chmod 666 merged/foo        # trigger copy-up
echo 2 > /proc/sys/vm/drop_caches  # avoid the dcache
md5sum merged/foo

>>
>> Fixes: 3e917cc305c6 ("erofs: make filesystem exportable")
>> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
>> ---
>>   fs/erofs/super.c | 51 ++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>> index cadec6b1b554..8f787c47e04d 100644
>> --- a/fs/erofs/super.c
>> +++ b/fs/erofs/super.c
>> @@ -511,24 +511,59 @@ static int erofs_fc_parse_param(struct fs_context *fc,
>>   	return 0;
>>   }
>>   
>> -static struct inode *erofs_nfs_get_inode(struct super_block *sb,
>> -					 u64 ino, u32 generation)
>> +static int erofs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> +			   struct inode *parent)
>>   {
>> -	return erofs_iget(sb, ino);
>> +	int len = parent ? 4 : 2;
>> +	erofs_nid_t nid;
>> +
>> +	if (*max_len < len) {
>> +		*max_len = len;
>> +		return FILEID_INVALID;
>> +	}
>> +
>> +	nid = EROFS_I(inode)->nid;
>> +	fh[0] = (u32)(nid >> 32);
>> +	fh[1] = (u32)(nid & 0xffffffff);
>> +
>> +	if (parent) {
>> +		nid = EROFS_I(parent)->nid;
>> +
>> +		fh[2] = (u32)(nid >> 32);
>> +		fh[3] = (u32)(nid & 0xffffffff);
>> +	}
>> +
>> +	*max_len = len;
>> +	return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>>   }
>>   
>>   static struct dentry *erofs_fh_to_dentry(struct super_block *sb,
>>   		struct fid *fid, int fh_len, int fh_type)
>>   {
>> -	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
>> -				    erofs_nfs_get_inode);
>> +	erofs_nid_t nid;
>> +
>> +	if ((fh_type != FILEID_INO64_GEN &&
>> +	     fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 2)
>> +		return NULL;
>> +
>> +	nid = (u64) fid->raw[0] << 32;
>> +	nid |= (u64) fid->raw[1];
>> +
> 
> Redundant new line.
> 
I will remove this in later version.

Thanks,
Hongbo

>> +	return d_obtain_alias(erofs_iget(sb, nid));
>>   }
>>   
>>   static struct dentry *erofs_fh_to_parent(struct super_block *sb,
>>   		struct fid *fid, int fh_len, int fh_type)
>>   {
>> -	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
>> -				    erofs_nfs_get_inode);
>> +	erofs_nid_t nid;
>> +
>> +	if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 4)
>> +		return NULL;
>> +
>> +	nid = (u64) fid->raw[2] << 32;
>> +	nid |= (u64) fid->raw[3];
>> +
> 
> Same here.
> 
> Thanks,
> Gao Xiang