Re: [PATCH v2] hfsplus: preserve FinderInfo when listing extended attributes

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Thu, 2026-10-22 at 05:40 -0400, Hengyu Liang wrote:
> Commit aef507847129 ("hfsplus: fix generic/062 xfstests failure")
> made
> hfsplus_listxattr() return zero when the inode has no records in the
> attributes B-tree.
> 
> However, osx.com.apple.FinderInfo is stored in the catalog record,
> and
> hfsplus_listxattr() has already added it to the result at that point.
> Setting the result to zero drops it. As of now, a file whose only
> extended attribute is FinderInfo gets an empty list on a volume with
> an attributes B-tree, which mkfs.hfsplus creates by default.
> getxattr()
> still returns the value.
> 
> The issue can be reproduced with:
> 
>     truncate -s 64M hfs.img
>     mkfs.hfsplus hfs.img
>     mount -o loop hfs.img /mnt
>     touch /mnt/file
>     v=0x54455854$(printf %056d 0)
>     setfattr -n osx.com.apple.FinderInfo -v $v /mnt/file
>     getfattr -m - /mnt/file
> 
> Without this patch, getfattr prints nothing. With this patch, it
> lists
> the FinderInfo entry.
> 
> This patch keeps the result of hfsplus_listxattr_finder_info() when
> no
> attributes B-tree records are found, as is already done when the
> volume has no attributes B-tree. A file without extended attributes
> still gets an empty list.
> 
> Fixes: aef507847129 ("hfsplus: fix generic/062 xfstests failure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
> ---
> Changes in v2:
> - Restructure the error check as suggested by Viacheslav Dubeyko. No
>   functional change.
> - v1:
> https://lore.kernel.org/r/20261021174534.482192-1-hengyul@cs.unc.edu
> 
> Tested in KVM with KASAN on a804c5742bff, without and with this
> patch:
> - The commands above, and a C test of listxattr() sizes and buffer
>   lengths: a file with only FinderInfo gives 0 without the patch and
>   25 with it; a file without extended attributes gives 0 on both.
> - generic/062: 10/10 passed on both kernels.
> - Full fstests (805 generic tests, 608 not run on both kernels): 186
>   passed and 11 failed without the patch, 187 and 10 with it. The
> only
>   difference is generic/795, which fails at random on both kernels
>   with fsck "Invalid volume file count" (1 of 11 runs without the
>   patch, 2 of 11 with it). generic/089 and a small test that only
>   creates files and unlinks them while open hit the same error on
> both
>   kernels. This looks like a separate race: hfsplus_file_release()
>   updates file_count without vh_mutex.
> 
> With this patch, getfattr above prints "osx.com.apple.FinderInf". The
> last character is lost by a separate bug in copy_name(), which
> Maurycy
> Pawlowski-Wieronski's patch fixes:
> https://lore.kernel.org/all/20260828-hfsplus-copy_name-fix-v1-1-75ed9f0f378c@maurycy.com/
> 
>  fs/hfsplus/xattr.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index 71364e093fa7..45571d6f3abd 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -837,13 +837,9 @@ ssize_t hfsplus_listxattr(struct dentry *dentry,
> char *buffer, size_t size)
>  
>  	err = hfsplus_find_attr(inode->i_sb, inode->i_ino, NULL,
> &fd);
>  	if (err) {
> -		if (err == -ENOENT || err == -ENODATA) {
> -			res = 0;
> -			goto end_listxattr;
> -		} else {
> +		if (err != -ENOENT && err != -ENODATA)
>  			res = err;
> -			goto end_listxattr;
> -		}
> +		goto end_listxattr;
>  	}
>  
>  	for (;;) {
> 
> base-commit: a804c5742bff224d4cb4d25dd383554d598be32b

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.