[musl] [PATCH] set tid address in fork

[musl] [PATCH] set tid address in fork

[changdiankang]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]

In pthrad_exit the exiting thread will hold the thread list lock,
and will unlock it in kernel by clear the tid address &__thread_list_lock.

A thread created by pthrad_create is created with clone syscall with
CLONE_CHILD_CLEARTID flag and &__thread_list_lock as child_tid
parameter. When the thread exits, the thread list lock can be woke up
(see man clone CLONE_CHILD_CLEARTID).

But in fork, the main thread is created with fork syscall or with
clone syscall without CLONE_CHILD_CLEARTID flag. The &__thread_list_lock
is not be set to tid address. So when the main thread exits, the thread
list lock can't be woke up by kernel. This may lead some problems when
other child threads try to hold this lock. For example, a child thread
blocks at waiting thread list lock when joining the main thread.

To fix this issue, we should set &__thread_list_lock to the main thread tid
address in fork.

[-- Attachment #2: 0001-set-tid-address-in-fork.patch --]
[-- Type: application/octet-stream, Size: 1685 bytes --]

From 237ed44f50b266ee6e1acbcbb433fe4264b0e36a Mon Sep 17 00:00:00 2001
From: Chang Diankang <changdiankang@huawei.com>
Date: Thu, 20 Jul 2023 17:25:01 +0800
Subject: [PATCH] set tid address in fork

In pthrad_exit the exiting thread will hold the thread list lock,
and will unlock it in kernel by clear the tid address &__thread_list_lock.

A thread created by pthrad_create is created with clone syscall with
CLONE_CHILD_CLEARTID flag and &__thread_list_lock as child_tid
parameter. When the thread exits, the thread list lock can be woke up
(see man clone CLONE_CHILD_CLEARTID).

But in fork, the main thread is created with fork syscall or with
clone syscall without CLONE_CHILD_CLEARTID flag. The &__thread_list_lock
is not be set to tid address. So when the main thread exits, the thread
list lock can't be woke up by kernel. This may leads some problems when
other child threads try to hold this lock. For example, a child thread
blocks at waiting thread list lock when joining the main thread.

To fix this issue, we should set &__thread_list_lock to the main thread tid
address in fork.

Signed-off-by: Chang Diankang <changdiankang@huawei.com>
---
 src/process/_Fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/process/_Fork.c b/src/process/_Fork.c
index da063868..a192e5a2 100644
--- a/src/process/_Fork.c
+++ b/src/process/_Fork.c
@@ -23,7 +23,7 @@ pid_t _Fork(void)
 #endif
 	if (!ret) {
 		pthread_t self = __pthread_self();
-		self->tid = __syscall(SYS_gettid);
+		self->tid = __syscall(SYS_set_tid_address, &__thread_list_lock);
 		self->robust_list.off = 0;
 		self->robust_list.pending = 0;
 		self->next = self->prev = self;
-- 
2.25.1

Re: [musl] [PATCH] set tid address in fork

[Alexey Izbyshev]

On 2023-07-20 16:53, changdiankang wrote:
> In pthrad_exit the exiting thread will hold the thread list lock,
> and will unlock it in kernel by clear the tid address 
> &__thread_list_lock.
> 
> A thread created by pthrad_create is created with clone syscall with
> CLONE_CHILD_CLEARTID flag and &__thread_list_lock as child_tid
> parameter. When the thread exits, the thread list lock can be woke up
> (see man clone CLONE_CHILD_CLEARTID).
> 
> But in fork, the main thread is created with fork syscall or with
> clone syscall without CLONE_CHILD_CLEARTID flag. The 
> &__thread_list_lock
> is not be set to tid address. So when the main thread exits, the thread
> list lock can't be woke up by kernel. This may lead some problems when
> other child threads try to hold this lock. For example, a child thread
> blocks at waiting thread list lock when joining the main thread.
> 
> To fix this issue, we should set &__thread_list_lock to the main thread 
> tid
> address in fork.

This bug has already been discovered, see 
https://www.openwall.com/lists/musl/2023/06/01/9.

Alexey

Re: [musl] [PATCH] set tid address in fork

[Rich Felker]

On Thu, Jul 20, 2023 at 05:22:07PM +0300, Alexey Izbyshev wrote:
> On 2023-07-20 16:53, changdiankang wrote:
> >In pthrad_exit the exiting thread will hold the thread list lock,
> >and will unlock it in kernel by clear the tid address
> >&__thread_list_lock.
> >
> >A thread created by pthrad_create is created with clone syscall with
> >CLONE_CHILD_CLEARTID flag and &__thread_list_lock as child_tid
> >parameter. When the thread exits, the thread list lock can be woke up
> >(see man clone CLONE_CHILD_CLEARTID).
> >
> >But in fork, the main thread is created with fork syscall or with
> >clone syscall without CLONE_CHILD_CLEARTID flag. The
> >&__thread_list_lock
> >is not be set to tid address. So when the main thread exits, the thread
> >list lock can't be woke up by kernel. This may lead some problems when
> >other child threads try to hold this lock. For example, a child thread
> >blocks at waiting thread list lock when joining the main thread.
> >
> >To fix this issue, we should set &__thread_list_lock to the main
> >thread tid
> >address in fork.
> 
> This bug has already been discovered, see
> https://www.openwall.com/lists/musl/2023/06/01/9.

Yes, and the fix is in an unpushed commit. Sorry! I'll review queue
and push in a bit.

Rich