[LTP] [PATCH v5] Add tls parameter and flag:CLONE_SETTLS cover for clone and clone3 syscall
Chunfu Wen
chwen@redhat.com
Fri May 9 05:57:36 CEST 2025
See inline comments.
Chunfu Wen
On Tue, May 6, 2025 at 6:46 PM Li Wang <liwang@redhat.com> wrote:
> Hi Chunfu,
>
> On Tue, Apr 29, 2025 at 4:50 PM chunfuwen via ltp <ltp@lists.linux.it>
> wrote:
>
>> tls parameter and related flag:CLONE_SETTLS are missed in the testing,
>> so add them into existed test case
>>
>> Signed-off-by: chunfuwen <chwen@redhat.com>
>> ---
>> Changes in v5:
>> - wrap duplicate code into one single methold
>> - remove duplicately malloc
>>
>> Changes in v4:
>> - remove riscv and loongarch definition
>>
>> Changes in v3:
>> - fix missing head file for x86
>>
>> Changes in v2:
>> - create separate files for clone and clone3
>>
>> ---
>> testcases/kernel/syscalls/clone/clone10.c | 172 ++++++++++++++++++++
>> testcases/kernel/syscalls/clone3/clone304.c | 152 +++++++++++++++++
>>
>
> We need to update .gitignore and runtest file as long as we add a new test.
>
>
>
>> 2 files changed, 324 insertions(+)
>> create mode 100644 testcases/kernel/syscalls/clone/clone10.c
>> create mode 100644 testcases/kernel/syscalls/clone3/clone304.c
>>
>> diff --git a/testcases/kernel/syscalls/clone/clone10.c
>> b/testcases/kernel/syscalls/clone/clone10.c
>> new file mode 100644
>> index 000000000..475bb2ece
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/clone/clone10.c
>> @@ -0,0 +1,172 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
>> + * Author: Chunfu Wen <chwen@redhat.com>
>> + */
>> +
>> +/*\
>> + * Add tls parameter and flag:CLONE_SETTLS cover for clone
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <stdlib.h>
>> +#include <stdio.h>
>> +#include <errno.h>
>> +#include <sched.h>
>> +#include <sys/wait.h>
>> +#if defined(__i386__)
>> +#include <asm/ldt.h>
>> +#endif
>> +
>> +#include "tst_test.h"
>> +#include "clone_platform.h"
>> +#include "lapi/syscalls.h"
>> +#include "lapi/futex.h"
>> +
>> +#define TLS_SIZE 4096
>> +#define TLS_ALIGN 16
>> +
>> +static pid_t ptid, ctid;
>> +static void *tls_ptr;
>> +static void *child_stack;
>> +static struct user_desc *tls_desc;
>> +
>> +/* TLS variable to validate in child */
>> +static __thread int tls_test_var = 12345;
>> +
>> +static int test_flags[] = {
>> + CLONE_SETTLS,
>> + CLONE_PARENT | CLONE_SETTLS,
>> + CLONE_CHILD_SETTID | CLONE_SETTLS,
>> + CLONE_PARENT_SETTID | CLONE_VM | CLONE_SETTLS,
>> + CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | CLONE_CHILD_CLEARTID |
>> CLONE_SETTLS | SIGCHLD,
>> + CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_VFORK | CLONE_SIGHAND |
>> CLONE_SETTLS,
>> +};
>>
>
> This does not look correct, from what I know, CLONE_SETTLS usually with
> CLONE_VM, CONE_FS, CONE_FILES, CLONE_SIGHAND, CLONE_THREAD
> in using, and it must have CLONE_THREAD | CLONE_VM | CLONE_SIGHAND
> as the minimal combination.
> [chwen] updated
>
>
>> +
>> +static void *allocate_tls_area(void)
>> +{
>> + void *tls_area = aligned_alloc(TLS_ALIGN, TLS_SIZE);
>> + if (!tls_area) {
>> + perror("aligned_alloc");
>> + exit(EXIT_FAILURE);
>> + }
>> + memset(tls_area, 0, TLS_SIZE);
>> + return tls_area;
>> +}
>> +
>> +static void init_tls(void)
>> +{
>> +#if defined(__x86_64__)
>> + /* x86-64: tls_ptr is the new %fs base address */
>> + tls_ptr = allocate_tls_area();
>> +
>> +#elif defined(__i386__)
>> + /* x86 32-bit: TLS is a struct user_desc */
>> + tls_ptr = allocate_tls_area();
>> + tls_desc = SAFE_MALLOC(sizeof(*tls_desc));
>> + memset(tls_desc, 0, sizeof(*tls_desc));
>> + tls_desc->entry_number = -1;
>> + tls_desc->base_addr = (unsigned long)tls_ptr;
>> + tls_desc->limit = TLS_SIZE;
>> + tls_desc->seg_32bit = 1;
>> + tls_desc->contents = 0;
>> + tls_desc->read_exec_only = 0;
>> + tls_desc->limit_in_pages = 0;
>> + tls_desc->seg_not_present = 0;
>> + tls_desc->useable = 1;
>> +#elif defined(__aarch64__) || defined(__powerpc64__) ||
>> defined(__s390x__)
>>
>
> The code below is the same as x86_64, why not combine them?
> [chwen] combined together
>
>
>> + /*
>> + * Other architectures: detect if dedicated TLS register is
>> available.
>> + * You don't manually touch TPIDR_EL0.
>> + * The kernel automatically writes it into the TPIDR_EL0 register
>> for the new thread after clone() succeeds.
>> + */
>> + tls_ptr = allocate_tls_area();
>> +#else
>> + tst_brk(TCONF, "This architecture does not support TLS");
>> +#endif
>> +}
>> +
>> +static void free_tls(void)
>> +{
>> +#if defined(__x86_64__)
>> + free(tls_ptr);
>> +
>> +#elif defined(__i386__)
>> + if (tls_desc) {
>> + free((void *)(uintptr_t)tls_desc->base_addr);
>> + free(tls_desc);
>> + }
>> +
>> +#elif defined(__aarch64__) || defined(__powerpc64__) ||
>> defined(__s390x__)
>> + free(tls_ptr);
>> +#endif
>> +}
>> +
>> +static void setup(void)
>> +{
>> + child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
>> + init_tls();
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> + free(child_stack);
>> + free_tls();
>> +}
>> +
>> +static int check_child(void *arg LTP_ATTRIBUTE_UNUSED)
>>
>
> Or, rename it to check_in_child sounds better?
> [chwen] renamed it to check_in_child
>
>
>> +{
>> + if (tls_test_var == 12345)
>> + tst_res(TPASS, "Child TLS variable has expected value");
>> + else
>> + tst_res(TFAIL, "Child TLS variable corrupted or not set");
>> +
>> + tst_syscall(__NR_exit, 0);
>>
>
> Why not exit(0) directly?
>
>
>
>> + return 0;
>>
>
> It never had a chance to come here, isn't it?
>
> [chwen] updated
>
>> +}
>> +
>> +static long clone_child(int flags)
>> +{
>> + TEST(ltp_clone7(flags, check_child, NULL, CHILD_STACK_SIZE,
>> + child_stack, &ptid, tls_ptr, &ctid));
>> +
>>
>
>
>> + if (TST_RET == -1 && TTERRNO == ENOSYS)
>> + tst_brk(TCONF, "clone parameters doesn't match");
>>
>
> The minimal supported kernel version is 4.4 for LTP, so we can drop those
> two lines safely.
> [chwen] remove those 2 lines
>
>> +
>> + if (TST_RET == -1)
>> + tst_brk(TBROK | TTERRNO, "clone() failed");
>> +
>> + return TST_RET;
>
> +}
>> +
>> +static void verify_tls(unsigned int i)
>> +{
>> + pid_t child;
>> + ctid = -1;
>> + struct timespec timeout = { 5 /* sec */, 0 };
>> + child = SAFE_FORK();
>> + if (!child) {
>>
>
>
>> + tst_syscall(__NR_getpid);
>>
>
> Useless code, plz remove it.
>
[chwen] remove it
>
> + clone_child(test_flags[i]);
>> + if (test_flags[i] & CLONE_THREAD) {
>> + if (syscall(SYS_futex, &ctid, FUTEX_WAIT, -1,
>> &timeout)) {
>> + if (errno != EWOULDBLOCK || ctid == -1) {
>> + tst_res(TFAIL | TERRNO,
>> + "futex failed, ctid: %d",
>> ctid);
>> + exit(0);
>> + }
>> + }
>> + }
>>
>
> Those nesting not elegant, we'd better avoid nesting more than 3 times.
> [chwen] remove them
>
>> + exit(0);
>> + }
>>
>
>
>> + sleep(1);
>>
>
> Any reason for sleeping 1 second here?
> [chwen] remove it
>
>
>> + tst_reap_children();
>>
>
> We need to report test result in the main loop, otherwise:
> [chwen] updated
> # ./clone10
> tst_test.c:1903: TINFO: LTP version: 20250130
> tst_test.c:1907: TINFO: Tested kernel: 6.14.4-300.fc42.x86_64 #1 SMP
> PREEMPT_DYNAMIC Fri Apr 25 15:43:38 UTC 2025 x86_64
> tst_kconfig.c:88: TINFO: Parsing kernel config
> '/lib/modules/6.14.4-300.fc42.x86_64/build/.config'
> tst_test.c:1720: TINFO: Overall timeout per run is 0h 00m 30s
> tst_test.c:1572: TBROK: Test 0 haven't reported results!
>
> Summary:
> passed 0
> failed 0
> broken 1
> skipped 0
> warnings 0
>
>
>
>> +}
>> +
>> +static struct tst_test test = {
>> + .tcnt = ARRAY_SIZE(test_flags),
>> + .test = verify_tls,
>> + .setup = setup,
>> + .cleanup = cleanup,
>> + .forks_child = 1
>> +};
>> diff --git a/testcases/kernel/syscalls/clone3/clone304.c
>> b/testcases/kernel/syscalls/clone3/clone304.c
>> new file mode 100644
>> index 000000000..6d307843b
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/clone3/clone304.c
>> @@ -0,0 +1,152 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
>> + * Author: Chunfu Wen <chwen@redhat.com>
>> + */
>> +
>> +/*\
>> + * Add tls parameter and flag:CLONE_SETTLS cover for clone3
>> + */
>> +
>> +#define _GNU_SOURCE
>> +
>> +#include <stdlib.h>
>> +#include <sys/wait.h>
>> +#if defined(__i386__)
>> +#include <asm/ldt.h>
>> +#endif
>> +
>> +#include "tst_test.h"
>> +#include "lapi/sched.h"
>> +#include "lapi/pidfd.h"
>> +
>> +#define TLS_SIZE 4096
>> +#define TLS_ALIGN 16
>> +
>> +static int pidfd, child_tid, parent_tid;
>> +static struct clone_args *args;
>> +static void *tls_ptr;
>> +static struct user_desc *tls_desc;
>> +
>> +/* TLS variable to validate in child */
>> +static __thread int tls_test_var = 54321;
>> +
>> +static int test_flags[] = {
>> + CLONE_FS | CLONE_SETTLS,
>> + CLONE_NEWPID | CLONE_SETTLS,
>> +};
>> +
>> +static void *allocate_tls_region(void)
>> +{
>> + void *tls_area = aligned_alloc(TLS_ALIGN, TLS_SIZE);
>> + if (!tls_area) {
>> + perror("aligned_alloc");
>> + exit(EXIT_FAILURE);
>> + }
>> + memset(tls_area, 0, TLS_SIZE);
>> + return tls_area;
>> +}
>> +
>> +static void initialize_tls(void)
>> +{
>> +#if defined(__x86_64__)
>> + /* x86-64: tls_ptr is the new %fs base address */
>> + tls_ptr = allocate_tls_region();
>> +
>> +#elif defined(__i386__)
>> + /* x86 32-bit: TLS is a struct user_desc */
>> + tls_ptr = allocate_tls_region();
>> + tls_desc = SAFE_MALLOC(sizeof(*tls_desc));
>> + memset(tls_desc, 0, sizeof(*tls_desc));
>> + tls_desc->entry_number = -1;
>> + tls_desc->base_addr = (unsigned long)tls_ptr;
>> + tls_desc->limit = TLS_SIZE;
>> + tls_desc->seg_32bit = 1;
>> + tls_desc->contents = 0;
>> + tls_desc->read_exec_only = 0;
>> + tls_desc->limit_in_pages = 0;
>> + tls_desc->seg_not_present = 0;
>> + tls_desc->useable = 1;
>> +
>> +#elif defined(__aarch64__) || defined(__powerpc64__) ||
>> defined(__s390x__)
>> + /*
>> + * Other architectures: detect if dedicated TLS register is
>> available.
>> + * You don't manually touch TPIDR_EL0.
>> + * The kernel automatically writes it into the TPIDR_EL0 register
>> for the new thread after clone() succeeds.
>> + */
>> + tls_ptr = allocate_tls_region();
>> +#else
>> + tst_brk(TCONF, "This architecture does not support TLS");
>> +#endif
>> +}
>> +
>> +static void free_tls(void)
>> +{
>> +#if defined(__x86_64__)
>> + free(tls_ptr);
>> +
>> +#elif defined(__i386__)
>> + if (tls_desc) {
>> + free((void *)(uintptr_t)tls_desc->base_addr);
>> + free(tls_desc);
>> + }
>> +
>> +#elif defined(__aarch64__) || defined(__powerpc64__) ||
>> defined(__s390x__)
>> + free(tls_ptr);
>> +#endif
>> +}
>> +
>> +static void do_child(void)
>> +{
>> + /* Validate TLS usage */
>> + if (tls_test_var == 54321) {
>> + tst_res(TPASS, "Child TLS variable has expected value");
>> + } else {
>> + tst_res(TFAIL, "Child TLS variable corrupted or not set");
>> + }
>> + exit(0);
>> +}
>> +
>> +static void run(unsigned int n)
>> +{
>> + int status;
>> + pid_t pid;
>> +
>> + args->flags = test_flags[n];
>> + args->pidfd = (uint64_t)(&pidfd);
>> + args->child_tid = (uint64_t)(&child_tid);
>> + args->parent_tid = (uint64_t)(&parent_tid);
>> + args->stack = 0;
>> + args->stack_size = 0;
>> + args->tls = (uint64_t)tls_ptr;
>> +
>> + TEST(pid = clone3(args, sizeof(*args)));
>> + if (pid < 0) {
>> + tst_res(TFAIL | TTERRNO, "clone3() failed (%d)", n);
>> + return;
>> + }
>> +
>> + if (!pid)
>> + do_child();
>> +
>> + SAFE_WAITPID(pid, &status, __WALL);
>> +}
>> +
>> +static void setup(void)
>> +{
>> + clone3_supported_by_kernel();
>> + initialize_tls();
>> +}
>> +
>> +static struct tst_test test = {
>> + .tcnt = ARRAY_SIZE(test_flags),
>> + .test = run,
>> + .setup = setup,
>> + .cleanup = free_tls,
>> + .needs_root = 1,
>> + .needs_checkpoints = 1,
>> + .bufs = (struct tst_buffers []) {
>> + {&args, .size = sizeof(*args)},
>> + {},
>> + }
>> +};
>> --
>> 2.43.5
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>>
>>
>
> --
> Regards,
> Li Wang
>
More information about the ltp
mailing list