Re: [LTP] [PATCH] lapi/tls: reserve pre-TCB space to avoid undefined behavior in clone10.c

[Petr Vorel <pvorel@suse.cz>]

Hi Changwei,

> Hi Petr,
> With the original upstream LTP,
> I ran clone10 -i 1000 on three machines (including AArch64 and AMD64), and
> it failed on all of them.

> This suggests there may be another bug that we still need to identify.

Yes, it's a separate bug, not relevant to your fix. I trigger it on x86_64.

Kind regards,
Petr

> Thank you very much for your invaluable information.
> Kind regards,
> Changwei
> *1. On an AArch64 cloud instance*
> ```sh
> azure@clone10-aarch64-kcp:~/orig/ltp$
> ./testcases/kernel/syscalls/clone/clone10-i1000
> clone10.c:68: TPASS:Parent(PID: 106163,TID:106163): TLS value correct: 100
> clone10.c:48: TINFO:Child(PID: 106163,TID:106200): TLS value set to: 101
> tst_test.c:1953: TBROK:TestkilledbySIGBUS!
> Summary:
> passed 36
> failed 0
> broken 1
> skipped 0
> warnings 0
> ```
> *2. On an AMD64 machine*
> ```sh
> ubuntu@ZBook:~/orig/ltp$ ./testcases/kernel/syscalls/clone/clone10-i1000
> clone10.c:48: TINFO:Child(PID: 125560,TID:125870): TLS value set to: 101
> clone10.c:68: TPASS:Parent(PID: 125560,TID:125560): TLS value correct: 100
> double freeorcorruption(out)
> clone10.c:48: TINFO:Child(PID: 125560,TID:125871): TLS value set to: 101
> clone10.c:68: TPASS:Parent(PID: 125560,TID:125560): TLS value correct: 100
> tst_test.c:1953: TBROK:TestkilledbySIGIOT/SIGABRT!
> Summary:
> passed 311
> failed 0
> broken 1
> skipped 0
> warnings 0
> ```
> *3. On an AArch64 machine*
> ```sh
> ubuntu@asus-pe100a:~/orig/ltp$
> ./testcases/kernel/syscalls/clone/clone10-i1000
> clone10.c:68: TPASS:Parent(PID: 158953,TID:158953): TLS value correct: 100
> clone10.c:48: TINFO:Child(PID: 158953,TID:159029): TLS value set to: 101
> tst_test.c:1953: TBROK:TestkilledbySIGSEGV!
> Summary:
> passed 75
> failed 0
> broken 1
> skipped 0
> warnings 0
> ```


> On 2/9/26 18:51, Petr Vorel wrote:
> > Hi Changwei,

> > > Allocate extra space before the TLS area to hold a struct pthread, ensuring
> > > THREAD_SELF->cancelhandling is initialized to 0. This prevents undefined
> > > behavior in __pthread_disable_asynccancel(), which is called at thread
> > > cancellation points such as write().
> > > Without this, touch_tls_in_child() could get stuck in tst_res().
> > LGTM, but I'd prefer others had a look on it.
> > Acked-by: Petr Vorel<pvorel@suse.cz>

> > BTW clone10.c segfaults w/a the patch when run with more iterations:

> > ./clone10 -i200
> > clone10.c:48: TINFO: Child (PID: 4271, TID: 4285): TLS value set to: 101
> > clone10.c:68: TPASS: Parent (PID: 4271, TID: 4271): TLS value correct: 100
> > clone10.c:48: TINFO: Child (PID: 4271, TID: 4286): TLS value set to: 101
> > clone10.c:68: TPASS: Parent (PID: 4271, TID: 4271): TLS value correct: 100
> > tst_test.c:1953: TBROK: Test killed by SIGSEGV!

> > Summary:
> > passed   15
> > failed   0
> > broken   1
> > skipped  0
> > warnings 0

> > Kind regards,
> > Petr

> > > (gdb) bt
> > >    0  futex_wait () at ../sysdeps/nptl/futex-internal.h:141
> > >    1  futex_wait_simple () at ../sysdeps/nptl/futex-internal.h:172
> > >    2  __libc_disable_asynccancel () at ../nptl/cancellation.c:100
> > >    3  __GI___libc_write () at ../sysdeps/unix/sysv/linux/write.c:26
> > >    4  __GI___libc_write () at ../sysdeps/unix/sysv/linux/write.c:24
> > >    5  print_result ()  at tst_test.c:387
> > >    6  tst_vres_ () at tst_test.c:401
> > >    7  tst_res_ () at tst_test.c:512
> > >    8  touch_tls_in_child (arg=<optimized out>) atclone10.c:48
> > >    9  thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone.S:78
> > > Signed-off-by: Changwei Zou<changwei.zou@canonical.com>
> > > ---
> > >   include/lapi/tls.h | 16 +++++++++++++---
> > >   1 file changed, 13 insertions(+), 3 deletions(-)
> > > diff --git a/include/lapi/tls.h b/include/lapi/tls.h
> > > index 468fe3086..7f2fa18a1 100644
> > > --- a/include/lapi/tls.h
> > > +++ b/include/lapi/tls.h
> > > @@ -22,6 +22,15 @@
> > >   #define TLS_SIZE 4096
> > >   #define TLS_ALIGN 16
> > > +/*
> > > + * Space allocated large enough to hold a struct pthread.
> > > + *
> > > + * Zero-initialized to ensure THREAD_SELF->cancelhandling starts at 0,
> > > + * avoiding undefined behavior (e.g., in clone10.c) in __pthread_disable_asynccancel(),
> > > + * which is called at thread cancellation points such as write().
> > > + */
> > > +#define TLS_PRE_TCB_SIZE (TLS_ALIGN * 256)
> > > +
> > >   #if defined(__x86_64__)
> > >   typedef struct {
> > >   	void *tcb;
> > > @@ -36,10 +45,11 @@ extern void *tls_ptr;
> > >   static inline void *allocate_tls_area(void)
> > >   {
> > > -	void *tls_area = aligned_alloc(TLS_ALIGN, TLS_SIZE);
> > > +	char *tls_area = aligned_alloc(TLS_ALIGN, TLS_PRE_TCB_SIZE + TLS_SIZE);
> > >   	if (!tls_area)
> > >   		tst_brk(TBROK | TERRNO, "aligned_alloc failed");
> > > -	memset(tls_area, 0, TLS_SIZE);
> > > +	memset(tls_area, 0, TLS_PRE_TCB_SIZE + TLS_SIZE);
> > > +	tls_area += TLS_PRE_TCB_SIZE;
> > >   #if defined(__x86_64__)
> > >   	tcb_t *tcb = (tcb_t *)tls_area;
> > > @@ -59,7 +69,7 @@ static inline void free_tls(void)
> > >   {
> > >   	usleep(10000);
> > >   	if (tls_ptr) {
> > > -		free(tls_ptr);
> > > +		free(((char *)tls_ptr) - TLS_PRE_TCB_SIZE);
> > >   		tls_ptr = NULL;
> > >   	}
> > >   }

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp