Re: [LTP] [PATCH v2 04/10] setxattr02: add setxattrat variant

Cyril Hrubis
2025-10-07T12:40:12Z

On Tue, Oct 07, 2025 at 08:46:56AM +0200, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> testcases/kernel/syscalls/setxattr/setxattr02.c | 79 +++++++++++++++++++------
> 1 file changed, 60 insertions(+), 19 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/setxattr/setxattr02.c b/testcases/kernel/syscalls/setxattr/setxattr02.c
> index 9f5f998da..b5042a0df 100644
> --- a/testcases/kernel/syscalls/setxattr/setxattr02.c
> +++ b/testcases/kernel/syscalls/setxattr/setxattr02.c
> @@ -19,6 +19,10 @@
> */
>
> #include "config.h"
> +#include "tst_test.h"
> +
> +#ifdef HAVE_SYS_XATTR_H
> +
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/sysmacros.h>
> @@ -30,12 +34,10 @@
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> -#ifdef HAVE_SYS_XATTR_H
> -# include <sys/xattr.h>
> -#endif
> -#include "tst_test.h"
> +#include "lapi/xattr.h"
> +#include "lapi/fcntl.h"
> +#include <sys/xattr.h>
>
> -#ifdef HAVE_SYS_XATTR_H
> #define XATTR_TEST_KEY "user.testkey"
> #define XATTR_TEST_VALUE "this is a test value"
> #define XATTR_TEST_VALUE_SIZE 20
> @@ -49,6 +51,8 @@
> #define BLK "setxattr02blk"
> #define SOCK "setxattr02sock"
>
> +static int tmpdir_fd = -1;
> +
> struct test_case {
> char *fname;
> char *key;
> @@ -120,39 +124,58 @@ static struct test_case tc[] = {
>
> static void verify_setxattr(unsigned int i)
> {
> + char *sysname;
> +
> /* some tests might require existing keys for each iteration */
> if (tc[i].needskeyset) {
> SAFE_SETXATTR(tc[i].fname, tc[i].key, tc[i].value, tc[i].size,
> - XATTR_CREATE);
> + XATTR_CREATE);
> }
>
> - TEST(setxattr(tc[i].fname, tc[i].key, tc[i].value, tc[i].size,
> - tc[i].flags));
> + if (tst_variant) {
> + sysname = "setxattrat";
> +
> + struct xattr_args args = {
> + .value = (uint64_t)tc[i].value,
> + .size = tc[i].size,
> + .flags = tc[i].flags,
> + };
> +
> + int at_flags = tc[i].needskeyset ? 0 : AT_SYMLINK_NOFOLLOW;

I do not get why this is needed.

In kernel:

SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
const char __user *, name, const void __user *, value,
size_t, size, int, flags)
{
return path_setxattrat(AT_FDCWD, pathname, 0, name, value, size, flags);
^
the setxattr() the
always sets the
at_flags to 0
}

So shouldn't setxattrat() just work the same if we pass 0 there?

It's the lsetxattr() syscall that passes AT_SYMLINK_NOFOLLOW:

SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
const char __user *, name, const void __user *, value,
size_t, size, int, flags)
{
return path_setxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
value, size, flags);
}

> static void setup(void)
> @@ -185,12 +208,30 @@ static void setup(void)
> SAFE_MKNOD(CHR, S_IFCHR | 0777, dev);
> SAFE_MKNOD(BLK, S_IFBLK | 0777, 0);
> SAFE_MKNOD(SOCK, S_IFSOCK | 0777, 0);
> +
> + tmpdir_fd = SAFE_OPEN(tst_tmpdir_path(), O_DIRECTORY);

This is memleak, on the top of that can't we just use the AT_FDCWD
instead? Or if you want to make sure that the syscall works with a real
fd we can do SAFE_OPEN(".", O_DIRECTORY) instead....

--
Cyril Hrubis
chrubis@suse.cz