Re: [PATCH] erofs: fix metabuf leak in shared xattr initialization

Hi Jia,

On 2026/5/20 11:42, Jia Zhu wrote:
> erofs_init_inode_xattrs() uses a local metabuf while reading the inline
> xattr header and the shared xattr id array.
> 
> It currently drops that metabuf from some error paths and from the success
> path, but the erofs_bread() failure while reading the shared xattr id array
> goes straight to out_unlock.
> 
> This became observable when file-backed metadata reads started calling
> rw_verify_area() before reusing or dropping the current metabuf.  Before
> that, the read_mapping_folio() failure path already dropped the old metabuf
> before returning an error.
> 
> Consolidate the local metabuf cleanup at out_unlock. erofs_put_metabuf()
> is a no-op if no page has been acquired, and this keeps all paths after
> taking EROFS_I_BL_XATTR_BIT covered by one cleanup site.
> 
> Fixes: 307210c262a2 ("erofs: verify metadata accesses for file-backed mounts")


Hmm, I don't think it's a correct "Fixes:", and
the commit message should be fixed too.

I think it's an issue due to missing erofs_put_metabuf()
in the erofs_init_inode_xattrs() error path instead.

I think
Fixes: bb88e8da0025 ("erofs: use meta buffers for xattr operations")

is a more proper "Fixes:".

Thanks,
Gao Xiang

> Signed-off-by: Jia Zhu <[email protected]>
> ---
>   fs/erofs/xattr.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/erofs/xattr.c b/fs/erofs/xattr.c
> index 41e311019a251..df7ea019526d7 100644
> --- a/fs/erofs/xattr.c
> +++ b/fs/erofs/xattr.c
> @@ -89,13 +89,11 @@ static int erofs_init_inode_xattrs(struct inode *inode)
>   	    vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) {
>   		erofs_err(sb, "invalid h_shared_count %u @ nid %llu",
>   			  vi->xattr_shared_count, vi->nid);
> -		erofs_put_metabuf(&buf);
>   		ret = -EFSCORRUPTED;
>   		goto out_unlock;
>   	}
>   	vi->xattr_shared_xattrs = kmalloc_objs(uint, vi->xattr_shared_count);
>   	if (!vi->xattr_shared_xattrs) {
> -		erofs_put_metabuf(&buf);
>   		ret = -ENOMEM;
>   		goto out_unlock;
>   	}
> @@ -112,12 +110,12 @@ static int erofs_init_inode_xattrs(struct inode *inode)
>   		}
>   		vi->xattr_shared_xattrs[i] = le32_to_cpu(*xattr_id);
>   	}
> -	erofs_put_metabuf(&buf);
>   
>   	/* paired with smp_mb() at the beginning of the function. */
>   	smp_mb();
>   	set_bit(EROFS_I_EA_INITED_BIT, &vi->flags);
>   out_unlock:
> +	erofs_put_metabuf(&buf);
>   	clear_and_wake_up_bit(EROFS_I_BL_XATTR_BIT, &vi->flags);
>   	return ret;
>   }