diff mbox series

[v2,09/10] Add setxattrat01 test

Message ID 20251007-xattrat-v2-9-bf458fa66358@suse.com
State Changes Requested
Delegated to: Andrea Cervesato
Headers show
Series setxattrat coverage | expand

Checks

Context Check Description
ltpci/debian_oldstable_gcc fail failure
ltpci/debian_stable_s390x-linux-gnu-gcc_s390x success success
ltpci/debian_stable_powerpc64le-linux-gnu-gcc_ppc64el success success
ltpci/debian_stable_aarch64-linux-gnu-gcc_arm64 success success
ltpci/ubuntu_jammy_gcc success success
ltpci/quay-io-centos-centos_stream9_gcc success success
ltpci/debian_stable_gcc success success
ltpci/ubuntu_bionic_gcc success success
ltpci/opensuse-leap_latest_gcc success success
ltpci/debian_testing_clang success success
ltpci/alpine_latest_gcc success success
ltpci/fedora_latest_clang success success
ltpci/debian_testing_gcc success success
ltpci/debian_oldstable_clang success success
ltpci/debian_stable_gcc success success
ltpci/opensuse-archive_42-2_gcc success success
ltpci/debian_oldstable_gcc success success

Commit Message

Andrea Cervesato Oct. 7, 2025, 6:47 a.m. UTC
From: Andrea Cervesato <andrea.cervesato@suse.com>

Test if setxattrat() syscall is correctly following symlink, setting
a xattr on a file.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/syscalls/setxattrat/.gitignore    |   1 +
 testcases/kernel/syscalls/setxattrat/Makefile      |   8 ++
 .../kernel/syscalls/setxattrat/setxattrat01.c      | 150 +++++++++++++++++++++
 3 files changed, 159 insertions(+)

Comments

Cyril Hrubis Oct. 8, 2025, 9:44 a.m. UTC | #1
Hi!
> +/*\
> + * [Description]
> + *
> + * Test if setxattrat() syscall is correctly following symlink, setting a
> + * xattr on a file.
> + *
> + * [Algorithm]
> + *
> + * - create a file and the symlink pointing to it
> + * - run setxattrat() on the symlink following the pointing file
> + * - verify that file xattr has been set
> + * - verify that symlink xattr has not been set

Shouldn't this be a variant of a setxattr() test?

> + * - run setxattrat() on the symlink with AT_SYMLINK_NOFOLLOW
> + * - verify that file xattr has not been set
> + * - verify that symlink xattr has been set
> + */

And this a variant of lsetxattr() test?
Andrea Cervesato Oct. 13, 2025, 7:54 a.m. UTC | #2
Hi!

On Wed Oct 8, 2025 at 11:44 AM CEST, Cyril Hrubis wrote:
> Hi!
>> +/*\
>> + * [Description]
>> + *
>> + * Test if setxattrat() syscall is correctly following symlink, setting a
>> + * xattr on a file.
>> + *
>> + * [Algorithm]
>> + *
>> + * - create a file and the symlink pointing to it
>> + * - run setxattrat() on the symlink following the pointing file
>> + * - verify that file xattr has been set
>> + * - verify that symlink xattr has not been set
>
> Shouldn't this be a variant of a setxattr() test?
>
>> + * - run setxattrat() on the symlink with AT_SYMLINK_NOFOLLOW
>> + * - verify that file xattr has not been set
>> + * - verify that symlink xattr has been set
>> + */
>
> And this a variant of lsetxattr() test?

This is the exact reason why I'm not yet 100% sure about coverage for
setxattrat, since most of the tests can be just variants of other
existing tests and most of the time we have code duplication.

After thinking carefully about this patch-set, I'm not sure to proceed
anymore, since the kernel itself is using setxattrat() to implement
setxattr() and lsetxattr().
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/setxattrat/.gitignore b/testcases/kernel/syscalls/setxattrat/.gitignore
new file mode 100644
index 000000000..e636401d7
--- /dev/null
+++ b/testcases/kernel/syscalls/setxattrat/.gitignore
@@ -0,0 +1 @@ 
+setxattrat01
diff --git a/testcases/kernel/syscalls/setxattrat/Makefile b/testcases/kernel/syscalls/setxattrat/Makefile
new file mode 100644
index 000000000..e582f727d
--- /dev/null
+++ b/testcases/kernel/syscalls/setxattrat/Makefile
@@ -0,0 +1,8 @@ 
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2025 SUSE LLC
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/setxattrat/setxattrat01.c b/testcases/kernel/syscalls/setxattrat/setxattrat01.c
new file mode 100644
index 000000000..86ad7503f
--- /dev/null
+++ b/testcases/kernel/syscalls/setxattrat/setxattrat01.c
@@ -0,0 +1,150 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Test if setxattrat() syscall is correctly following symlink, setting a
+ * xattr on a file.
+ *
+ * [Algorithm]
+ *
+ * - create a file and the symlink pointing to it
+ * - run setxattrat() on the symlink following the pointing file
+ * - verify that file xattr has been set
+ * - verify that symlink xattr has not been set
+ * - run setxattrat() on the symlink with AT_SYMLINK_NOFOLLOW
+ * - verify that file xattr has not been set
+ * - verify that symlink xattr has been set
+ */
+
+#include "tst_test.h"
+#include "lapi/xattr.h"
+
+#include <sys/xattr.h>
+
+#define FNAME "ltp_file"
+#define SNAME "ltp_symbolic_file"
+#define XATTR_TEST_KEY "trusted.ltptestkey"
+#define XATTR_TEST_VALUE "ltprulez"
+#define XATTR_TEST_VALUE_SIZE 8
+
+static struct xattr_args *args;
+static int tmpdir_fd = -1;
+
+static struct tcase {
+	char *dst_set;
+	char *dst_noset;
+	int at_flags;
+} tcases[] = {
+	{
+		.dst_set = FNAME,
+		.dst_noset = SNAME,
+		.at_flags = 0,
+	},
+	{
+		.dst_set = SNAME,
+		.dst_noset = FNAME,
+		.at_flags = AT_SYMLINK_NOFOLLOW,
+	}
+};
+
+static void expect_xattr(const char *fname)
+{
+	int ret;
+	char buff[XATTR_TEST_VALUE_SIZE];
+
+	tst_res(TINFO, "Check if %s has xattr", fname);
+
+	memset(args, 0, sizeof(*args));
+	memset(buff, 0, XATTR_TEST_VALUE_SIZE);
+
+	args->value = (uint64_t)buff;
+	args->size = XATTR_TEST_VALUE_SIZE;
+
+	ret = SAFE_GETXATTRAT(tmpdir_fd, fname, AT_SYMLINK_NOFOLLOW,
+		XATTR_TEST_KEY, args, sizeof(*args));
+
+	TST_EXP_EQ_LI(ret, XATTR_TEST_VALUE_SIZE);
+	TST_EXP_EQ_LI(args->size, XATTR_TEST_VALUE_SIZE);
+	TST_EXP_EQ_LI(args->flags, 0);
+	TST_EXP_EQ_STRN((char *)args->value, XATTR_TEST_VALUE, XATTR_TEST_VALUE_SIZE);
+}
+
+static void expect_no_xattr(const char *fname)
+{
+	char buff[XATTR_TEST_VALUE_SIZE];
+
+	tst_res(TINFO, "Check if %s has no xattr", fname);
+
+	memset(args, 0, sizeof(*args));
+	memset(buff, 0, XATTR_TEST_VALUE_SIZE);
+
+	args->value = (uint64_t)buff;
+	args->size = 0;
+
+	TST_EXP_FAIL(tst_syscall(__NR_getxattrat, tmpdir_fd, fname,
+		AT_SYMLINK_NOFOLLOW, XATTR_TEST_KEY, args, sizeof(*args)),
+		ENODATA);
+
+	TST_EXP_EQ_LI(args->size, 0);
+	TST_EXP_EQ_LI(args->flags, 0);
+	TST_EXP_EQ_STRN((char *)args->value, "\0", 1);
+}
+
+static void run(unsigned int i)
+{
+	struct tcase *tc = &tcases[i];
+
+	args->value = (uint64_t)XATTR_TEST_VALUE;
+	args->size = XATTR_TEST_VALUE_SIZE;
+	args->flags = XATTR_CREATE;
+
+	tst_res(TINFO, "Setting xattr '%s' in %s (flags=%s)",
+		XATTR_TEST_KEY, SNAME,
+		!tc->at_flags ? "0" : "AT_SYMLINK_NOFOLLOW");
+
+	SAFE_SETXATTRAT(tmpdir_fd, SNAME, tc->at_flags, XATTR_TEST_KEY,
+		 args, sizeof(*args));
+
+	expect_xattr(tc->dst_set);
+	expect_no_xattr(tc->dst_noset);
+
+	SAFE_REMOVEXATTRAT(tmpdir_fd, tc->dst_set, tc->at_flags,
+		XATTR_TEST_KEY);
+}
+
+static void setup(void)
+{
+	char *tmpdir;
+
+	tmpdir = tst_tmpdir_path();
+	tmpdir_fd = SAFE_OPEN(tmpdir, O_DIRECTORY);
+
+	SAFE_TOUCH(FNAME, 0777, NULL);
+	SAFE_SYMLINK(FNAME, SNAME);
+}
+
+static void cleanup(void)
+{
+	if (tmpdir_fd != -1)
+		SAFE_CLOSE(tmpdir_fd);
+
+	SAFE_UNLINK(SNAME);
+	SAFE_UNLINK(FNAME);
+}
+
+static struct tst_test test = {
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.tcnt = ARRAY_SIZE(tcases),
+	.bufs = (struct tst_buffers []) {
+		{&args, .size = sizeof(struct xattr_args)},
+		{},
+	}
+};