[PATCH v2] hfsplus: fix truncated xattr names in listxattr()

[Hengyu Liang]

From: Maurycy Pawlowski-Wieronski <maurycy@xxxxxxxxxxx>

Commit 7dcbf17e3f91 ("hfsplus: refactor copy_name to not use strncpy")
made copy_name() build names in the osx. namespace with scnprintf().

However, the size passed to scnprintf() leaves no room for the
terminating NUL, so the last character of the name is dropped. As of
now, listxattr() returns every osx. name without its last character,
and these names cannot be passed back to getxattr().

The issue can be reproduced with:

    truncate -s 64M hfs.img
    mkfs.hfsplus hfs.img
    mount -o loop hfs.img /mnt
    touch /mnt/file
    setfattr -n osx.test -v 1 /mnt/file
    getfattr -m - /mnt/file

Without this patch, getfattr lists "osx.com.apple.FinderInf" and
"osx.tes". With this patch, it lists "osx.com.apple.FinderInfo" and
"osx.test".

This patch adds room for the NUL to the size passed to scnprintf().

Fixes: 7dcbf17e3f91 ("hfsplus: refactor copy_name to not use strncpy")
Cc: stable@xxxxxxxxxxxxxxx # v6.10+
Signed-off-by: Maurycy Pawlowski-Wieronski <maurycy@xxxxxxxxxxx>
Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
---
Changes in v2:
- Rewrite the commit message. No code change.
- v1: https://lore.kernel.org/r/20260828-hfsplus-copy_name-fix-v1-1-75ed9f0f378c@xxxxxxxxxxx

Maurycy, v1 got no follow-up, so I am resending it as Viacheslav asked
in the thread below, with you kept as the author. Let me know if you
would rather send it yourself.
https://lore.kernel.org/r/dc187921249a3f6716b829624853e64c10321cd1.camel@xxxxxxxxxxx

Answers to the questions on v1:
- generic/377 passes with and without this patch. It only uses user.
  names, which go through the strscpy() branch. That branch already
  passes name_len + 1. No xfstests test uses osx. names.
- Why + 1 helps: the prefix and the name are name_len +
  XATTR_MAC_OSX_PREFIX_LEN characters long, and scnprintf() writes at
  most size - 1 characters before the NUL. Zeroing the buffer first
  does not help, as the last character is never written.
- Why the write stays in bounds: both callers check that the buffer
  has room for name_len() bytes, which covers the prefix, the name and
  the NUL, before calling copy_name(). This is also the number of bytes
  copy_name() wrote before commit 7dcbf17e3f91.
- All xfstests: see the full run below.

Tested in KVM with KASAN on a804c5742bff, without and with this patch:
- The commands above, and a C test that compares listxattr() with the
  names set on files and a directory: short, long (up to 127
  characters) and non-ASCII osx. names, FinderInfo, and user. and
  trusted. names. Without the patch, every osx. name loses its last
  character, fewer bytes are copied than the size query returns, a
  buffer one byte short does not get -ERANGE, and getxattr() fails on
  the listed osx. names. With it, the names match, the copied length
  equals the size query, and a buffer one byte short gets -ERANGE.
  KASAN reported nothing with exact-size buffers.
- generic/377 and generic/062: 5/5 passed on both kernels.
- Full fstests (805 generic tests, 608 not run on both kernels): 188
  passed and 9 failed without the patch, 187 and 10 with it. The only
  difference is generic/617, which fails on both kernels most of the
  time (19 of 20 reruns without the patch, 18 of 20 with it).
  fsck.hfsplus -fn reported no errors.

 fs/hfsplus/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
index 71364e093fa7..470f090b2699 100644
--- a/fs/hfsplus/xattr.c
+++ b/fs/hfsplus/xattr.c
@@ -515,8 +515,8 @@ static ssize_t copy_name(char *buffer, const char *xattr_name, size_t name_len)
 	memset(buffer, 0, name_len);
 
 	if (!is_known_namespace(xattr_name)) {
-		len = scnprintf(buffer, name_len + XATTR_MAC_OSX_PREFIX_LEN,
-				 "%s%s", XATTR_MAC_OSX_PREFIX, xattr_name);
+		len = scnprintf(buffer, name_len + XATTR_MAC_OSX_PREFIX_LEN + 1,
+				"%s%s", XATTR_MAC_OSX_PREFIX, xattr_name);
 	} else {
 		len = strscpy(buffer, xattr_name, name_len + 1);
 		if (len < 0) {

base-commit: a804c5742bff224d4cb4d25dd383554d598be32b
-- 
2.53.0