Re: [PATCH] erofs-utils: fsck: extract zero-length xattr values

[Gao Xiang <xiang@kernel.org>]

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
> 
>