From: Maurycy Pawlowski-Wieronski 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@vger.kernel.org # v6.10+ Signed-off-by: Maurycy Pawlowski-Wieronski Signed-off-by: Hengyu Liang --- Changes in v2: - Rewrite the commit message. No code change. - v1: https://lore.kernel.org/r/20260828-hfsplus-copy_name-fix-v1-1-75ed9f0f378c@maurycy.com 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@dubeyko.com 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