[LTP] [PATCH v11] Add tls parameter and flag:CLONE_SETTLS cover for clone and clone3 syscall
Chunfu Wen
chwen@redhat.com
Tue Jul 22 04:59:58 CEST 2025
Hello,
Thank you for your review.
See inline comments for my reply.
Also updated patch v12 was sent out as well
Chunfu Wen
On Fri, Jul 18, 2025 at 11:26 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > include/lapi/tls.h | 102 ++++++++++++++++++
> > runtest/syscalls | 2 +
> > testcases/kernel/syscalls/clone/.gitignore | 1 +
> > testcases/kernel/syscalls/clone/clone10.c | 98 ++++++++++++++++++
> > testcases/kernel/syscalls/clone3/.gitignore | 1 +
> > testcases/kernel/syscalls/clone3/clone304.c | 108 ++++++++++++++++++++
>
> There a few couple of things to fix. Please run make check in the
> directories this patch adds files (include/lapi/ syscalls/clone/ and
> syscalls/clone3/) and fix the errors and warnings reported for the newly
> added files.
> [chunfu] Updated as comment.
> > 6 files changed, 312 insertions(+)
> > create mode 100644 include/lapi/tls.h
> > create mode 100644 testcases/kernel/syscalls/clone/clone10.c
> > create mode 100644 testcases/kernel/syscalls/clone3/clone304.c
> >
> > diff --git a/include/lapi/tls.h b/include/lapi/tls.h
> > new file mode 100644
> > index 000000000..f433fc392
> > --- /dev/null
> > +++ b/include/lapi/tls.h
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
> > + * Author: Chunfu Wen <chwen@redhat.com>
> > + */
> > +
> > +/*\
> > + * tls init/alloc/free common functions
> > + */
> > +
> > +#ifndef _LAPI_TLS_H
> > +#define _LAPI_TLS_H
> > +
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +
> > +#if defined(__i386__)
> > +#include <asm/ldt.h>
> > +#endif
> > +
> > +#include "tst_test.h"
> > +
> > +#define TLS_SIZE 4096
> > +#define TLS_ALIGN 16
> > +
> > +#if defined(__x86_64__)
> > +// Structure mimicking glibc's TCB to be simplified for x86_64
> > +typedef struct {
> > + void *tcb;
> > + void *dtv;
> > + void *self;
> > + int multiple_threads;
> > + char padding[64];
> > +} tcb_t;
> > +#endif
> > +
> > +// Global pointers for TLS management
> > +extern void *tls_ptr;
> > +extern struct user_desc *tls_desc;
> > +
> > +static inline void *allocate_tls_area(void)
> > +{
> > + void *tls_area = aligned_alloc(TLS_ALIGN, TLS_SIZE);
> > + if (!tls_area)
> > + tst_brk(TBROK | TERRNO, "aligned_alloc failed");
> > + memset(tls_area, 0, TLS_SIZE);
> > +
> > +#if defined(__x86_64__)
> > + // Set up a minimal TCB for x86_64
> > + tcb_t *tcb = (tcb_t *)tls_area;
> > + tcb->tcb = tls_area;
> > + tcb->self = tls_area;
> > + tcb->multiple_threads = 1;
> > +#endif
> > + return tls_area;
> > +}
> > +
> > +static inline void init_tls(void)
> > +{
> > +#if defined(__x86_64__) || defined(__aarch64__) || defined(__s390x__)
> > + tls_ptr = allocate_tls_area();
> > +
> > +#elif defined(__i386__)
> > + 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;
> > +
> > +#else
> > + tst_brk(TCONF, "Unsupported architecture for TLS");
> > +#endif
> > +}
> > +
> > +static inline void free_tls(void)
> > +{
> > + usleep(1000);
>
> Why is this sleep needed here?
>
> [chunfu] This is neccessary. If remove this, the test will fail on some
> linux distributions, such as limited computing resources, with error:TBROK:
> Test killed by SIGSEGV
> +#if defined(__x86_64__) || defined(__aarch64__) || defined(__s390x__)
> > + if (tls_ptr) {
> > + free(tls_ptr);
> > + tls_ptr = NULL;
> > + }
> > +#elif defined(__i386__)
> > + if (tls_desc) {
> > + free((void *)(uintptr_t)tls_desc->base_addr);
> > + free(tls_desc);
> > + tls_desc = NULL;
> > + }
> > +#endif
> > +}
> > +
> > +#endif // _LAPI_TLS_H
> > diff --git a/runtest/syscalls b/runtest/syscalls
> > index 844ae7a13..10f64270a 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -122,10 +122,12 @@ clone06 clone06
> > clone07 clone07
> > clone08 clone08
> > clone09 clone09
> > +clone10 clone10
> >
> > clone301 clone301
> > clone302 clone302
> > clone303 clone303
> > +clone304 clone304
> >
> > close01 close01
> > close02 close02
> > diff --git a/testcases/kernel/syscalls/clone/.gitignore
> b/testcases/kernel/syscalls/clone/.gitignore
> > index 900cac19c..adfb8257d 100644
> > --- a/testcases/kernel/syscalls/clone/.gitignore
> > +++ b/testcases/kernel/syscalls/clone/.gitignore
> > @@ -7,3 +7,4 @@
> > /clone07
> > /clone08
> > /clone09
> > +/clone10
> > diff --git a/testcases/kernel/syscalls/clone/clone10.c
> b/testcases/kernel/syscalls/clone/clone10.c
> > new file mode 100644
> > index 000000000..b0899a6b8
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/clone/clone10.c
> > @@ -0,0 +1,98 @@
> > +// 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
>
>
> This should explain what the test is doing better. As far as I can tell
> we do:
>
> Test that in a thread started by clone() that runs in the same address
> space (CLONE_VM) but with a different TLS (CLONE_SETTLS) writtes to a
> thread local variables are not propagated back from the cloned thread.
>
> [chunfu] Updated as comment
> > + */
> > +
> > +#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/tls.h"
> > +
> > +#define TLS_EXP 100
> > +
> > +#ifndef ARCH_SET_FS
> > +#define ARCH_SET_FS 0x1002
> > +#endif
> > +
> > +// Global pointers for TLS management
> > +void *tls_ptr = NULL;
> > +struct user_desc *tls_desc = NULL;
> > +
> > +static __thread int tls_var = 0;
> > +
> > +static char *child_stack;
> > +static volatile int child_done = 0;
> > +
> > +static int flags = CLONE_THREAD | CLONE_VM | CLONE_FS | CLONE_FILES |
> CLONE_SIGHAND | CLONE_SETTLS;
> > +
> > +static int touch_tls_in_child(void *arg LTP_ATTRIBUTE_UNUSED)
> > +{
> > +#if defined(__x86_64__)
> > + // Set the %fs register to point to the TCB
> > + if (syscall(SYS_arch_prctl, ARCH_SET_FS, tls_ptr) == -1) {
> > + exit(EXIT_FAILURE);
> > + }
>
> So this is stil needed despite the tst_ptr passed to the kernel in the
> clone() call?
>
[chunfu] On some linux distributions, the test will fail if without it.
>
> Can we move this to a function in the lapi header as well? Something as
> "child_init_tls()" and call it here?
> [chunfu] It looks like it is not proper to move into lapi. While the TLS
> setup and teardown are general, the actual testing of the TLS variable
> within the child context often involves test-specific variables and logic.
> Since this method consume tls_var, making tls_var (a __thread variable)
> global and extern across different files, especially when its purpose is
> specific to how a child process interacts with TLS in a test, can lead to
> complexity and potential issues with thread-local storage behavior across
> different executables or shared libraries
> > +#endif
> > + tls_var = TLS_EXP + 1;
> > + tst_res(TINFO, "Child (PID: %d, TID: %d): TLS value set to: %d",
> getpid(), gettid(), tls_var);
> > +
> > + TST_CHECKPOINT_WAKE(0);
> > + free_tls();
> > + return 0;
>
> Shouldn't we call the __NR_exit here as well?
> [chunfu] Added as comment
> > +}
> > +
> > +static void verify_tls(void)
> > +{
> > + tls_var = TLS_EXP;
> > +
> > + TEST(ltp_clone7(flags, touch_tls_in_child, NULL, CHILD_STACK_SIZE,
> child_stack, NULL, tls_ptr, NULL));
> > +
> > + if (TST_RET == -1)
> > + tst_brk(TBROK | TTERRNO, "clone() failed");
> > +
> > + TST_CHECKPOINT_WAIT(0);
> > +
> > + if (TLS_EXP == tls_var) {
> > + tst_res(TPASS,
> > + "Parent (PID: %d, TID: %d): TLS value correct: %d",
> > + getpid(), gettid(), tls_var);
> > + } else {
> > + tst_res(TFAIL,
> > + "Parent (PID: %d, TID: %d): TLS value mismatch:
> got %d, expected %d",
> > + getpid(), gettid(), tls_var, TLS_EXP);
> > + }
> > +}
> > +
> > +static void setup(void)
> > +{
> > + child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> > + init_tls();
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > + free(child_stack);
> > +}
> > +
> > +static struct tst_test test = {
> > + .setup = setup,
> > + .cleanup = cleanup,
> > + .needs_checkpoints = 1,
> > + .test_all = verify_tls,
> > +};
> > diff --git a/testcases/kernel/syscalls/clone3/.gitignore
> b/testcases/kernel/syscalls/clone3/.gitignore
> > index 10369954b..e9b5312f4 100644
> > --- a/testcases/kernel/syscalls/clone3/.gitignore
> > +++ b/testcases/kernel/syscalls/clone3/.gitignore
> > @@ -1,3 +1,4 @@
> > clone301
> > clone302
> > clone303
> > +clone304
> > diff --git a/testcases/kernel/syscalls/clone3/clone304.c
> b/testcases/kernel/syscalls/clone3/clone304.c
> > new file mode 100644
> > index 000000000..d45ac2395
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/clone3/clone304.c
> > @@ -0,0 +1,108 @@
> > +// 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
>
> The description here should be the same as in the previous one but it
> should say clone3 instead of clone
> [chunfu] Updated as comment.
>
[chunfu] One more comment related to Parent/Child Sync with TST_CHECKPOINT_,
in clone304.c
it doesn't work as expected with the below error: TBROK: Test killed by
SIGSEGV!
So I keep the previous solution using single volatile int child_done.
> > + */
> > +
> > +#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"
> > +#include "lapi/tls.h"
> > +
> > +#define TLS_EXP 100
> > +
> > +#define CHILD_STACK_SIZE (1024*1024)
> > +
> > +// Global pointers for TLS management
> > +void *tls_ptr = NULL;
> > +struct user_desc *tls_desc = NULL;
> > +
> > +static int pidfd, child_tid, parent_tid;
> > +static struct clone_args *args;
> > +static void *child_stack;
> > +
> > +/* TLS variable to validate in child */
> > +static __thread int tls_var = 0;
> > +
> > +static int test_flags = CLONE_THREAD | CLONE_VM | CLONE_FS |
> CLONE_FILES | CLONE_SIGHAND | CLONE_SETTLS;
> > +
> > +static int check_tls_in_child(void)
> > +{
> > + tls_var = TLS_EXP + 1;
> > + tst_res(TINFO, "Child (PID: %d, TID: %d): TLS value set to: %d",
> getpid(), gettid(), tls_var);
> > +
> > + TST_CHECKPOINT_WAKE(0);
> > + tst_syscall(__NR_exit, 0);
> > + return 0;
> > +}
> > +
> > +static void run(void)
> > +{
> > + pid_t pid;
> > + tls_var = TLS_EXP;
> > +
> > + args->flags = test_flags;
> > + args->pidfd = (uint64_t)(&pidfd);
> > + args->child_tid = (uint64_t)(&child_tid);
> > + args->parent_tid = (uint64_t)(&parent_tid);
> > + args->stack = (uint64_t)(child_stack);
> > + args->stack_size = CHILD_STACK_SIZE;
> > + args->tls = (uint64_t)tls_ptr;
> > +
> > + TEST(pid = clone3(args, sizeof(*args)));
> > +
> > + if (TST_RET == -1)
> > + tst_brk(TBROK | TTERRNO, "clone() failed");
> > +
> > + if (!pid)
> > + check_tls_in_child();
> > +
> > + TST_CHECKPOINT_WAIT(0);
> > +
> > + if (TLS_EXP == tls_var) {
> > + tst_res(TPASS,
> > + "Parent (PID: %d, TID: %d): TLS value correct: %d",
> > + getpid(), gettid(), tls_var);
> > + } else {
> > + tst_res(TFAIL,
> > + "Parent (PID: %d, TID: %d): TLS value mismatch:
> got %d, expected %d",
> > + getpid(), gettid(), tls_var, TLS_EXP);
> > + }
> > +}
> > +
> > +static void setup(void)
> > +{
> > + clone3_supported_by_kernel();
> > + child_stack = SAFE_MALLOC(CHILD_STACK_SIZE);
> > + init_tls();
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > + free(child_stack);
> > + free_tls();
> > +}
> > +
> > +static struct tst_test test = {
> > + .setup = setup,
> > + .cleanup = cleanup,
> > + .needs_checkpoints = 1,
> > + .test_all = run,
> > + .bufs = (struct tst_buffers []) {
> > + {&args, .size = sizeof(*args)},
> > + {},
> > + }
> > +};
> > --
> > 2.43.5
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
>
More information about the ltp
mailing list