All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] erofs-utils: fsck: extract zero-length xattr values
@ 2026-11-04  3:52 Wenyuan Xu
  2026-11-04  8:36 ` Gao Xiang
  0 siblings, 1 reply; 3+ messages in thread
From: Wenyuan Xu @ 2026-11-04  3:52 UTC (permalink / raw)
  To: xiang, linux-erofs; +Cc: sereno.xu

A zero return value from erofs_getxattr() denotes an empty attribute,
but the extraction path treats it as an error.

Accept zero-length values and allocate at least one byte for the value
buffer, avoiding dependence on malloc(0) behavior.

Signed-off-by: Wenyuan Xu <sereno.xu@deepseek.com>
---
 fsck/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index e54c523..9db5ae9 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -458,14 +458,15 @@ static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
 		size_t size = 0;
 
 		ret = erofs_getxattr(inode, key, NULL, 0);
-		if (ret <= 0) {
+		if (ret < 0) {
 			DBG_BUGON(1);
 			erofs_err("failed to get xattr value size of `%s` @ nid %llu",
 				  key, inode->nid | 0ULL);
 			break;
 		}
 		size = ret;
-		value = malloc(size);
+		/* Empty xattrs are valid */
+		value = malloc(size ? size : 1);
 		if (!value) {
 			ret = -ENOMEM;
 			break;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] erofs-utils: fsck: extract zero-length xattr values
  2026-11-04  3:52 [PATCH] erofs-utils: fsck: extract zero-length xattr values Wenyuan Xu
@ 2026-11-04  8:36 ` Gao Xiang
  2026-11-04  9:04   ` Wenyuan Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Gao Xiang @ 2026-11-04  8:36 UTC (permalink / raw)
  To: Wenyuan Xu; +Cc: xiang, linux-erofs

Hi Wenyuan,

On Wed, Nov 04, 2026 at 03:52:08AM +0000, Wenyuan Xu wrote:
> A zero return value from erofs_getxattr() denotes an empty attribute,
> but the extraction path treats it as an error.
> 
> Accept zero-length values and allocate at least one byte for the value
> buffer, avoiding dependence on malloc(0) behavior.
> 
> Signed-off-by: Wenyuan Xu <sereno.xu@deepseek.com>

According to
https://linux.die.net/man/2/getxattr

Return Value
On success, a positive number is returned indicating the
size of the extended attribute value.

So 0-sized xattrs are not a valid number or here is a mistake.

In principle, filesystems accept 0-size xattrs, and
overlayfs uses it (see ovl_path_check_origin_xattr()).

I think EROFS should support 0-size xattrs, but here
we shouldn't expand to 1.

I applied the following diff (please find if it works or not):

diff --git a/fsck/main.c b/fsck/main.c
index 8e3ae52..1a36ef8 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -458,25 +458,33 @@ static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
 		size_t size = 0;
 
 		ret = erofs_getxattr(inode, key, NULL, 0);
-		if (ret <= 0) {
+		if (ret < 0) {
 			DBG_BUGON(1);
 			erofs_err("failed to get xattr value size of `%s` @ nid %llu",
 				  key, inode->nid | 0ULL);
 			break;
 		}
 		size = ret;
-		value = malloc(size);
-		if (!value) {
-			ret = -ENOMEM;
-			break;
-		}
-		ret = erofs_getxattr(inode, key, value, size);
-		if (ret < 0) {
-			erofs_err("failed to get xattr `%s` @ nid %llu, because of `%s`", key,
-				  inode->nid | 0ULL, erofs_strerror(ret));
-			free(value);
-			break;
+
+		/* Empty xattrs are allowed */
+		if (!size) {
+			value = NULL;
+		} else {
+			value = malloc(size);
+			if (!value) {
+				ret = -ENOMEM;
+				break;
+			}
+
+			ret = erofs_getxattr(inode, key, value, size);
+			if (ret < 0) {
+				erofs_err("failed to get xattr `%s` @ nid %llu, because of `%s`", key,
+					  inode->nid | 0ULL, erofs_strerror(ret));
+				free(value);
+				break;
+			}
 		}
+
 		if (fsckcfg.extract_path)
 			ret = erofs_sys_lsetxattr(fsckcfg.extract_path, key,
 						  value, size);

Thanks,
Gao Xiang

> ---
>  fsck/main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index e54c523..9db5ae9 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -458,14 +458,15 @@ static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
>  		size_t size = 0;
>  
>  		ret = erofs_getxattr(inode, key, NULL, 0);
> -		if (ret <= 0) {
> +		if (ret < 0) {
>  			DBG_BUGON(1);
>  			erofs_err("failed to get xattr value size of `%s` @ nid %llu",
>  				  key, inode->nid | 0ULL);
>  			break;
>  		}
>  		size = ret;
> -		value = malloc(size);
> +		/* Empty xattrs are valid */
> +		value = malloc(size ? size : 1);
>  		if (!value) {
>  			ret = -ENOMEM;
>  			break;
> -- 
> 2.43.0
> 
> 


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] erofs-utils: fsck: extract zero-length xattr values
  2026-11-04  8:36 ` Gao Xiang
@ 2026-11-04  9:04   ` Wenyuan Xu
  0 siblings, 0 replies; 3+ messages in thread
From: Wenyuan Xu @ 2026-11-04  9:04 UTC (permalink / raw)
  To: xiang, linux-erofs


在 2026/11/4 16:36, Gao Xiang 写道:
> Hi Wenyuan,
>
> On Wed, Nov 04, 2026 at 03:52:08AM +0000, Wenyuan Xu wrote:
>> A zero return value from erofs_getxattr() denotes an empty attribute,
>> but the extraction path treats it as an error.
>>
>> Accept zero-length values and allocate at least one byte for the value
>> buffer, avoiding dependence on malloc(0) behavior.
>>
>> Signed-off-by: Wenyuan Xu <sereno.xu@deepseek.com>
> According to
> https://linux.die.net/man/2/getxattr
>
> Return Value
> On success, a positive number is returned indicating the
> size of the extended attribute value.
>
> So 0-sized xattrs are not a valid number or here is a mistake.
>
> In principle, filesystems accept 0-size xattrs, and
> overlayfs uses it (see ovl_path_check_origin_xattr()).
>
> I think EROFS should support 0-size xattrs, but here
> we shouldn't expand to 1.
>
> I applied the following diff (please find if it works or not):


LGTM and it works for me.

Thanks.


Wenyuan Xu



> diff --git a/fsck/main.c b/fsck/main.c
> index 8e3ae52..1a36ef8 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -458,25 +458,33 @@ static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
>   		size_t size = 0;
>   
>   		ret = erofs_getxattr(inode, key, NULL, 0);
> -		if (ret <= 0) {
> +		if (ret < 0) {
>   			DBG_BUGON(1);
>   			erofs_err("failed to get xattr value size of `%s` @ nid %llu",
>   				  key, inode->nid | 0ULL);
>   			break;
>   		}
>   		size = ret;
> -		value = malloc(size);
> -		if (!value) {
> -			ret = -ENOMEM;
> -			break;
> -		}
> -		ret = erofs_getxattr(inode, key, value, size);
> -		if (ret < 0) {
> -			erofs_err("failed to get xattr `%s` @ nid %llu, because of `%s`", key,
> -				  inode->nid | 0ULL, erofs_strerror(ret));
> -			free(value);
> -			break;
> +
> +		/* Empty xattrs are allowed */
> +		if (!size) {
> +			value = NULL;
> +		} else {
> +			value = malloc(size);
> +			if (!value) {
> +				ret = -ENOMEM;
> +				break;
> +			}
> +
> +			ret = erofs_getxattr(inode, key, value, size);
> +			if (ret < 0) {
> +				erofs_err("failed to get xattr `%s` @ nid %llu, because of `%s`", key,
> +					  inode->nid | 0ULL, erofs_strerror(ret));
> +				free(value);
> +				break;
> +			}
>   		}
> +
>   		if (fsckcfg.extract_path)
>   			ret = erofs_sys_lsetxattr(fsckcfg.extract_path, key,
>   						  value, size);
>
> Thanks,
> Gao Xiang
>
>> ---
>>   fsck/main.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fsck/main.c b/fsck/main.c
>> index e54c523..9db5ae9 100644
>> --- a/fsck/main.c
>> +++ b/fsck/main.c
>> @@ -458,14 +458,15 @@ static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
>>   		size_t size = 0;
>>   
>>   		ret = erofs_getxattr(inode, key, NULL, 0);
>> -		if (ret <= 0) {
>> +		if (ret < 0) {
>>   			DBG_BUGON(1);
>>   			erofs_err("failed to get xattr value size of `%s` @ nid %llu",
>>   				  key, inode->nid | 0ULL);
>>   			break;
>>   		}
>>   		size = ret;
>> -		value = malloc(size);
>> +		/* Empty xattrs are valid */
>> +		value = malloc(size ? size : 1);
>>   		if (!value) {
>>   			ret = -ENOMEM;
>>   			break;
>> -- 
>> 2.43.0
>>
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-04  9:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04  3:52 [PATCH] erofs-utils: fsck: extract zero-length xattr values Wenyuan Xu
2026-11-04  8:36 ` Gao Xiang
2026-11-04  9:04   ` Wenyuan Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.