Skip to content

Commit 413466f

Browse files
dubeykodubeyko
committed
hfsplus: fix generic/020 xfstests failure
The xfstests' test-case generic/020 fails to execute correctly: FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.15.0-rc4+ #8 SMP PREEMPT_DYNAMIC Thu May 1 16:43:22 PDT 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/020 _check_generic_filesystem: filesystem on /dev/loop50 is inconsistent (see xfstests-dev/results//generic/020.full for details) *** add lots of attributes *** check *** MAX_ATTRS attribute(s) +/mnt/test/attribute_12286: Numerical result out of range *** -1 attribute(s) *** remove lots of attributes ... (Run 'diff -u /xfstests-dev/tests/generic/020.out /xfstests-dev/results//generic/020.out.bad' to see the entire diff) The generic/020 creates more than 100 xattrs and gives its the names user.attribute_<number> (for example, user.attribute_101). As the next step, listxattr() is called with the goal to check the correctness of xattrs creation. However, it was issue in hfsplus_listxattr() logic. This method re-uses the fd.key->attr.key_name.unicode and strbuf buffers in the loop without re-initialization. As a result, part of the previous name could still remain in the buffers. For example, user.attribute_101 could be processed before user.attribute_54. The issue resulted in formation the name user.attribute_541 instead of user.attribute_54. This patch adds initialization of fd.key->attr.key_name.unicode and strbuf buffers before calling hfs_brec_goto() method that prepare next name in the buffer. HFS+ logic supports only inline xattrs. Such extended attributes can store values not bigger than 3802 bytes [1]. This limitation requires correction of generic/020 logic. Finally, generic/020 can be executed without any issue: sudo ./check generic/020 FSTYP -- hfsplus PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.19.0-rc1+ #44 SMP PREEMPT_DYNAMIC Mon Dec 22 15:39:00 PST 2025 MKFS_OPTIONS -- /dev/loop51 MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch generic/020 31s ... 38s Ran: generic/020 Passed all 1 tests [1] https://elixir.bootlin.com/linux/v6.19-rc2/source/include/linux/hfs_common.h#L626 Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> cc: Yangtao Li <frank.li@vivo.com> cc: linux-fsdevel@vger.kernel.org Link: https://lore.kernel.org/r/20251224002810.1137139-1-slava@dubeyko.com Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
1 parent 9a8c4ad commit 413466f

2 files changed

Lines changed: 70 additions & 14 deletions

File tree

‎fs/hfsplus/attributes.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,10 @@ static int hfsplus_attr_build_record(hfsplus_attr_entry *entry, int record_type,
117117
entry->inline_data.record_type = cpu_to_be32(record_type);
118118
if (size <= HFSPLUS_MAX_INLINE_DATA_SIZE)
119119
len = size;
120-
else
120+
else {
121+
hfs_dbg("value size %zu is too big\n", size);
121122
return HFSPLUS_INVALID_ATTR_RECORD;
123+
}
122124
entry->inline_data.length = cpu_to_be16(len);
123125
memcpy(entry->inline_data.raw_bytes, value, len);
124126
/*
@@ -238,7 +240,11 @@ int hfsplus_create_attr(struct inode *inode,
238240
inode->i_ino,
239241
value, size);
240242
if (entry_size == HFSPLUS_INVALID_ATTR_RECORD) {
241-
err = -EINVAL;
243+
if (size > HFSPLUS_MAX_INLINE_DATA_SIZE)
244+
err = -E2BIG;
245+
else
246+
err = -EINVAL;
247+
hfs_dbg("unable to store value: err %d\n", err);
242248
goto failed_create_attr;
243249
}
244250

@@ -250,8 +256,10 @@ int hfsplus_create_attr(struct inode *inode,
250256
}
251257

252258
err = hfs_brec_insert(&fd, entry_ptr, entry_size);
253-
if (err)
259+
if (err) {
260+
hfs_dbg("unable to store value: err %d\n", err);
254261
goto failed_create_attr;
262+
}
255263

256264
hfsplus_mark_inode_dirty(inode, HFSPLUS_I_ATTR_DIRTY);
257265

‎fs/hfsplus/xattr.c‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -345,17 +345,21 @@ int __hfsplus_setxattr(struct inode *inode, const char *name,
345345
if (err)
346346
goto end_setxattr;
347347
err = hfsplus_create_attr(inode, name, value, size);
348-
if (err)
348+
if (err) {
349+
hfs_dbg("unable to store value: err %d\n", err);
349350
goto end_setxattr;
351+
}
350352
} else {
351353
if (flags & XATTR_REPLACE) {
352354
pr_err("cannot replace xattr\n");
353355
err = -EOPNOTSUPP;
354356
goto end_setxattr;
355357
}
356358
err = hfsplus_create_attr(inode, name, value, size);
357-
if (err)
359+
if (err) {
360+
hfs_dbg("unable to store value: err %d\n", err);
358361
goto end_setxattr;
362+
}
359363
}
360364

361365
cat_entry_type = hfs_bnode_read_u16(cat_fd.bnode, cat_fd.entryoffset);
@@ -392,25 +396,32 @@ int __hfsplus_setxattr(struct inode *inode, const char *name,
392396
return err;
393397
}
394398

395-
static int name_len(const char *xattr_name, int xattr_name_len)
399+
static size_t name_len(const char *xattr_name, size_t xattr_name_len)
396400
{
397-
int len = xattr_name_len + 1;
401+
size_t len = xattr_name_len + 1;
398402

399403
if (!is_known_namespace(xattr_name))
400404
len += XATTR_MAC_OSX_PREFIX_LEN;
401405

402406
return len;
403407
}
404408

405-
static ssize_t copy_name(char *buffer, const char *xattr_name, int name_len)
409+
static ssize_t copy_name(char *buffer, const char *xattr_name, size_t name_len)
406410
{
407411
ssize_t len;
408412

409-
if (!is_known_namespace(xattr_name))
413+
memset(buffer, 0, name_len);
414+
415+
if (!is_known_namespace(xattr_name)) {
410416
len = scnprintf(buffer, name_len + XATTR_MAC_OSX_PREFIX_LEN,
411417
"%s%s", XATTR_MAC_OSX_PREFIX, xattr_name);
412-
else
418+
} else {
413419
len = strscpy(buffer, xattr_name, name_len + 1);
420+
if (len < 0) {
421+
pr_err("fail to copy name: err %zd\n", len);
422+
len = 0;
423+
}
424+
}
414425

415426
/* include NUL-byte in length for non-empty name */
416427
if (len >= 0)
@@ -423,16 +434,26 @@ int hfsplus_setxattr(struct inode *inode, const char *name,
423434
const char *prefix, size_t prefixlen)
424435
{
425436
char *xattr_name;
437+
size_t xattr_name_len =
438+
NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1;
426439
int res;
427440

428-
xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1,
429-
GFP_KERNEL);
441+
hfs_dbg("ino %lu, name %s, prefix %s, prefixlen %zu, "
442+
"value %p, size %zu\n",
443+
inode->i_ino, name ? name : NULL,
444+
prefix ? prefix : NULL, prefixlen,
445+
value, size);
446+
447+
xattr_name = kmalloc(xattr_name_len, GFP_KERNEL);
430448
if (!xattr_name)
431449
return -ENOMEM;
432450
strcpy(xattr_name, prefix);
433451
strcpy(xattr_name + prefixlen, name);
434452
res = __hfsplus_setxattr(inode, xattr_name, value, size, flags);
435453
kfree(xattr_name);
454+
455+
hfs_dbg("finished: res %d\n", res);
456+
436457
return res;
437458
}
438459

@@ -579,6 +600,10 @@ ssize_t hfsplus_getxattr(struct inode *inode, const char *name,
579600
int res;
580601
char *xattr_name;
581602

603+
hfs_dbg("ino %lu, name %s, prefix %s\n",
604+
inode->i_ino, name ? name : NULL,
605+
prefix ? prefix : NULL);
606+
582607
xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1,
583608
GFP_KERNEL);
584609
if (!xattr_name)
@@ -589,6 +614,9 @@ ssize_t hfsplus_getxattr(struct inode *inode, const char *name,
589614

590615
res = __hfsplus_getxattr(inode, xattr_name, value, size);
591616
kfree(xattr_name);
617+
618+
hfs_dbg("finished: res %d\n", res);
619+
592620
return res;
593621

594622
}
@@ -679,8 +707,11 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
679707
struct hfs_find_data fd;
680708
struct hfsplus_attr_key attr_key;
681709
char *strbuf;
710+
size_t strbuf_size;
682711
int xattr_name_len;
683712

713+
hfs_dbg("ino %lu\n", inode->i_ino);
714+
684715
if ((!S_ISREG(inode->i_mode) &&
685716
!S_ISDIR(inode->i_mode)) ||
686717
HFSPLUS_IS_RSRC(inode))
@@ -698,8 +729,9 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
698729
return err;
699730
}
700731

701-
strbuf = kzalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
702-
XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
732+
strbuf_size = NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN +
733+
XATTR_MAC_OSX_PREFIX_LEN + 1;
734+
strbuf = kzalloc(strbuf_size, GFP_KERNEL);
703735
if (!strbuf) {
704736
res = -ENOMEM;
705737
goto out;
@@ -746,13 +778,20 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
746778
res += name_len(strbuf, xattr_name_len);
747779
} else if (can_list(strbuf)) {
748780
if (size < (res + name_len(strbuf, xattr_name_len))) {
781+
pr_err("size %zu, res %zd, name_len %zu\n",
782+
size, res,
783+
name_len(strbuf, xattr_name_len));
749784
res = -ERANGE;
750785
goto end_listxattr;
751786
} else
752787
res += copy_name(buffer + res,
753788
strbuf, xattr_name_len);
754789
}
755790

791+
memset(fd.key->attr.key_name.unicode, 0,
792+
sizeof(fd.key->attr.key_name.unicode));
793+
memset(strbuf, 0, strbuf_size);
794+
756795
if (hfs_brec_goto(&fd, 1))
757796
goto end_listxattr;
758797
}
@@ -761,6 +800,9 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
761800
kfree(strbuf);
762801
out:
763802
hfs_find_exit(&fd);
803+
804+
hfs_dbg("finished: res %zd\n", res);
805+
764806
return res;
765807
}
766808

@@ -773,6 +815,9 @@ static int hfsplus_removexattr(struct inode *inode, const char *name)
773815
int is_xattr_acl_deleted;
774816
int is_all_xattrs_deleted;
775817

818+
hfs_dbg("ino %lu, name %s\n",
819+
inode->i_ino, name ? name : NULL);
820+
776821
if (!HFSPLUS_SB(inode->i_sb)->attr_tree)
777822
return -EOPNOTSUPP;
778823

@@ -833,6 +878,9 @@ static int hfsplus_removexattr(struct inode *inode, const char *name)
833878

834879
end_removexattr:
835880
hfs_find_exit(&cat_fd);
881+
882+
hfs_dbg("finished: err %d\n", err);
883+
836884
return err;
837885
}
838886

0 commit comments

Comments
 (0)