[LTP] [PATCH v2] Add listxattr04 reproducer
Andrea Cervesato
andrea.cervesato@suse.com
Mon Jul 7 11:43:16 CEST 2025
Hi Wei,
On 7/7/25 9:42 PM, Wei Gao wrote:
> On Thu, Jul 03, 2025 at 03:51:33PM +0200, Andrea Cervesato wrote:
>> From: Andrea Cervesato <andrea.cervesato@suse.com>
>>
>> Test reproducer for a bug introduced in 8b0ba61df5a1 ("fs/xattr.c: fix
>> simple_xattr_list to always include security.* xattrs").
>>
>> Bug can be reproduced when SELinux and ACL are activated on inodes as
>> following:
>>
>> $ touch testfile
>> $ setfacl -m u:myuser:rwx testfile
>> $ getfattr -dm. /tmp/testfile
>> Segmentation fault (core dumped)
>>
>> The reason why this happens is that simple_xattr_list() always includes
>> security.* xattrs without resetting error flag after
>> security_inode_listsecurity(). This results into an incorrect length of the
>> returned xattr name if POSIX ACL is also applied on the inode.
>>
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>> Reproducer for https://lore.kernel.org/linux-fsdevel/m1wm9qund4.fsf@gmail.com/T/
>> ---
>> Changes in v2:
>> - only check if SELinux is up and running
>> - Link to v1: https://lore.kernel.org/r/20250703-xattr_bug_repr-v1-1-5dcf5dde8b61@suse.com
>> ---
>> testcases/kernel/syscalls/listxattr/.gitignore | 1 +
>> testcases/kernel/syscalls/listxattr/Makefile | 2 +
>> testcases/kernel/syscalls/listxattr/listxattr04.c | 133 ++++++++++++++++++++++
>> 3 files changed, 136 insertions(+)
>>
>> diff --git a/testcases/kernel/syscalls/listxattr/.gitignore b/testcases/kernel/syscalls/listxattr/.gitignore
>> index be0675a6df0080d176d53d70194442bbc9ed376c..0d672b6ea5eec03aab37ee89316c56e24356c1d9 100644
>> --- a/testcases/kernel/syscalls/listxattr/.gitignore
>> +++ b/testcases/kernel/syscalls/listxattr/.gitignore
>> @@ -1,3 +1,4 @@
>> /listxattr01
>> /listxattr02
>> /listxattr03
>> +/listxattr04
>> diff --git a/testcases/kernel/syscalls/listxattr/Makefile b/testcases/kernel/syscalls/listxattr/Makefile
>> index c2f84b1590fc24a7a98f890ea7706771d944aa79..e96bb3fa4c2c6b14b8d2bc8fd4c475e4789d72fe 100644
>> --- a/testcases/kernel/syscalls/listxattr/Makefile
>> +++ b/testcases/kernel/syscalls/listxattr/Makefile
>> @@ -6,4 +6,6 @@ top_srcdir ?= ../../../..
>>
>> include $(top_srcdir)/include/mk/testcases.mk
>>
>> +listxattr04: LDLIBS += $(ACL_LIBS)
>> +
>> include $(top_srcdir)/include/mk/generic_leaf_target.mk
>> diff --git a/testcases/kernel/syscalls/listxattr/listxattr04.c b/testcases/kernel/syscalls/listxattr/listxattr04.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..af80e0c609d5931132b3bf16b28805577b2a853d
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/listxattr/listxattr04.c
>> @@ -0,0 +1,133 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2025 Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * Test reproducer for a bug introduced in 8b0ba61df5a1 ("fs/xattr.c: fix
>> + * simple_xattr_list to always include security.* xattrs").
>> + *
>> + * Bug can be reproduced when SELinux and ACL are activated on inodes as
>> + * following:
>> + *
>> + * $ touch testfile
>> + * $ setfacl -m u:myuser:rwx testfile
>> + * $ getfattr -dm. /tmp/testfile
>> + * Segmentation fault (core dumped)
>> + *
>> + * The reason why this happens is that simple_xattr_list() always includes
>> + * security.* xattrs without resetting error flag after
>> + * security_inode_listsecurity(). This results into an incorrect length of the
>> + * returned xattr name if POSIX ACL is also applied on the inode.
>> + */
>> +
>> +#include "config.h"
>> +#include "tst_test.h"
>> +
>> +#if defined(HAVE_SYS_XATTR_H) && defined(HAVE_LIBACL)
>> +
>> +#include "lapi/lsm.h"
>> +
>> +#include <pwd.h>
>> +#include <sys/acl.h>
>> +#include <sys/xattr.h>
>> +
>> +#define ACL_PERM "u::rw-,u:root:rwx,g::r--,o::r--,m::rwx"
>> +#define TEST_FILE "test.bin"
>> +
>> +static acl_t acl;
>> +
>> +static void verify_xattr(const int size)
>> +{
>> + char buf[size];
>> +
>> + memset(buf, 0, sizeof(buf));
>> +
>> + if (listxattr(TEST_FILE, buf, size) == -1) {
>> + if (errno != ERANGE)
>> + tst_brk(TBROK | TERRNO, "listxattr() error");
>> +
>> + tst_res(TFAIL, "listxattr() failed to read attributes length: ERANGE");
>> + return;
> Why ERANGE is Checked Separately?
> Such as tst_res(TFAIL | TERRNO, "listxattr() error") will also report
> errno.
ERANGE is the error we get if kernel is affected by this bug.
>> + }
>> +
>> + tst_res(TPASS, "listxattr() correctly read attributes length");
>> +}
>> +
>> +static void run(void)
>> +{
>> + int size;
>> +
>> + size = listxattr(TEST_FILE, NULL, 0);
>> + if (size == -1)
>> + tst_brk(TBROK | TERRNO, "listxattr() error");
>> +
>> + verify_xattr(size);
>> +}
>> +
>> +static int selinux_enabled(void)
>> +{
>> + uint32_t lsm_num;
>> + uint64_t ids[32];
>> + uint32_t page_size;
>> + int available = 0;
>> +
>> + page_size = SAFE_SYSCONF(_SC_PAGESIZE);
>> +
>> + lsm_num = lsm_list_modules(ids, &page_size, 0);
>> + if (!lsm_num)
>> + return 0;
>> +
>> + for (uint32_t i = 0; i < lsm_num; i++) {
>> + if (ids[i] == LSM_ID_SELINUX) {
>> + available = 1;
>> + break;
>> + }
>> + }
>> +
>> + return available;
>> +}
>> +
>> +static void setup(void)
>> +{
>> + int res;
>> +
>> + if (!selinux_enabled())
> Mabye we can use is_selinux_enabled instead of self build function?
I seen it, but it's related to the libselinux header files used to
communicate with the LSM.
We might not have libselinux headers, but SELinux enabled anyway.
>> + tst_brk(TCONF, "SELinux is not running");
>> +
>> + SAFE_TOUCH(TEST_FILE, 0644, NULL);
>> +
>> + acl = acl_from_text(ACL_PERM);
>> + if (!acl)
>> + tst_brk(TBROK | TERRNO, "acl_from_text() failed");
>> +
>> + res = acl_set_file(TEST_FILE, ACL_TYPE_ACCESS, acl);
>> + if (res == -1) {
>> + if (errno == EOPNOTSUPP)
>> + tst_brk(TCONF | TERRNO, "acl_set_file()");
>> +
>> + tst_brk(TBROK | TERRNO, "acl_set_file(%s) failed", TEST_FILE);
>> + }
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> + if (acl)
>> + acl_free(acl);
> unlink(TEST_FILE);
>> +}
>> +
>> +static struct tst_test test = {
>> + .test_all = run,
>> + .setup = setup,
>> + .cleanup = cleanup,
>> + .needs_root = 1,
>> + .needs_tmpdir = 1,
>> + .tags = (const struct tst_tag[]) {
>> + {"linux-git", "800d0b9b6a8b"},
>> + {}
>> + }
>> +};
>> +
>> +#else /* HAVE_SYS_XATTR_H && HAVE_LIBACL */
>> + TST_TEST_TCONF("<sys/xattr.h> or <sys/acl.h> does not exist.");
>> +#endif
>>
>> ---
>> base-commit: a908cff70f9389c2dd2bf535976cb179bfa8f340
>> change-id: 20250702-xattr_bug_repr-5873b84792fb
>>
>> Best regards,
>> --
>> Andrea Cervesato <andrea.cervesato@suse.com>
>>
> Thanks for your patch, some minor comments see above.
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
- Andrea
More information about the ltp
mailing list