1. Move thread list flags into new separate member, and atomically

   put DEAD thread on GC list, this closes a race between pthread_join
   and thr_cleanup.
2. Introduce a mutex to protect tcb initialization, tls allocation and
   deallocation code in rtld seems no lock protection or it is broken,
   under stress testing, memory is corrupted.

Reviewed by: deischen
patch partly provided by: deischen

5 files changed, 45 insertions, 24 deletions

diff --git a/lib/libkse/thread/thr_create.c b/lib/libkse/thread/thr_create.c
index 54bcd3b..03f467b 100644
--- a/lib/libkse/thread/thr_create.c
+++ b/lib/libkse/thread/thr_create.c
@@ -234,6 +234,7 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr,
 			new_thread->specific_data_count = 0;
 			new_thread->cleanup = NULL;
 			new_thread->flags = 0;
+			new_thread->tlflags = 0;
 			new_thread->continuation = NULL;
 			new_thread->wakeup_time.tv_sec = -1;
 			new_thread->lock_switch = 0;
diff --git a/lib/libkse/thread/thr_find_thread.c b/lib/libkse/thread/thr_find_thread.c
index e8d9fcb..5a64640 100644
--- a/lib/libkse/thread/thr_find_thread.c
+++ b/lib/libkse/thread/thr_find_thread.c
@@ -90,7 +90,7 @@ _thr_ref_delete(struct pthread *curthread, struct pthread *thread)
 		if (curthread != NULL)
 			curthread->critical_count--;
 		if ((thread->refcount == 0) &&
-		    (thread->flags & THR_FLAGS_GC_SAFE) != 0)
+		    (thread->tlflags & TLFLAGS_GC_SAFE) != 0)
 			THR_GCLIST_ADD(thread);
 		KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
 		_kse_critical_leave(crit);
diff --git a/lib/libkse/thread/thr_kern.c b/lib/libkse/thread/thr_kern.c
index 6e0003a..5d7fabb 100644
--- a/lib/libkse/thread/thr_kern.c
+++ b/lib/libkse/thread/thr_kern.c
@@ -139,6 +139,9 @@ LIST_HEAD(thread_hash_head, pthread);
 static struct thread_hash_head	thr_hashtable[THREAD_HASH_QUEUES];
 #define	THREAD_HASH(thrd)	((unsigned long)thrd % THREAD_HASH_QUEUES)
 
+/* Lock for thread tcb constructor/destructor */
+static pthread_mutex_t		_tcb_mutex;
+
 #ifdef DEBUG_THREAD_KERN
 static void	dump_queues(struct kse *curkse);
 #endif
@@ -166,7 +169,7 @@ static void	thr_resume_check(struct pthread *curthread, ucontext_t *ucp,
 		    struct pthread_sigframe *psf);
 static int	thr_timedout(struct pthread *thread, struct timespec *curtime);
 static void	thr_unlink(struct pthread *thread);
-static void	thr_destroy(struct pthread *thread);
+static void	thr_destroy(struct pthread *curthread, struct pthread *thread);
 static void	thread_gc(struct pthread *thread);
 static void	kse_gc(struct pthread *thread);
 static void	kseg_gc(struct pthread *thread);
@@ -240,7 +243,7 @@ _kse_single_thread(struct pthread *curthread)
 			_thr_stack_free(&thread->attr);
 			if (thread->specific != NULL)
 				free(thread->specific);
-			thr_destroy(thread);
+			thr_destroy(curthread, thread);
 		}
 	}
 
@@ -285,14 +288,14 @@ _kse_single_thread(struct pthread *curthread)
 	/* Free the free threads. */
 	while ((thread = TAILQ_FIRST(&free_threadq)) != NULL) {
 		TAILQ_REMOVE(&free_threadq, thread, tle);
-		thr_destroy(thread);
+		thr_destroy(curthread, thread);
 	}
 	free_thread_count = 0;
 
 	/* Free the to-be-gc'd threads. */
 	while ((thread = TAILQ_FIRST(&_thread_gc_list)) != NULL) {
 		TAILQ_REMOVE(&_thread_gc_list, thread, gcle);
-		thr_destroy(thread);
+		thr_destroy(curthread, thread);
 	}
 	TAILQ_INIT(&gc_ksegq);
 	_gc_count = 0;
@@ -381,6 +384,7 @@ _kse_init(void)
 		if (_lock_init(&_thread_list_lock, LCK_ADAPTIVE,
 		    _kse_lock_wait, _kse_lock_wakeup) != 0)
 			PANIC("Unable to initialize thread list lock");
+		_pthread_mutex_init(&_tcb_mutex, NULL);
 		active_kse_count = 0;
 		active_kseg_count = 0;
 		_gc_count = 0;
@@ -1207,7 +1211,6 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
 		thread->kseg = _kse_initial->k_kseg;
 		thread->kse = _kse_initial;
 	}
-	thread->flags |= THR_FLAGS_GC_SAFE;
 
 	/*
 	 * We can't hold the thread list lock while holding the
@@ -1216,6 +1219,7 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
 	KSE_SCHED_UNLOCK(curkse, curkse->k_kseg);
 	DBG_MSG("Adding thread %p to GC list\n", thread);
 	KSE_LOCK_ACQUIRE(curkse, &_thread_list_lock);
+	thread->tlflags |= TLFLAGS_GC_SAFE;
 	THR_GCLIST_ADD(thread);
 	KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
 	if (sys_scope) {
@@ -1255,7 +1259,7 @@ thread_gc(struct pthread *curthread)
 	/* Check the threads waiting for GC. */
 	for (td = TAILQ_FIRST(&_thread_gc_list); td != NULL; td = td_next) {
 		td_next = TAILQ_NEXT(td, gcle);
-		if ((td->flags & THR_FLAGS_GC_SAFE) == 0)
+		if ((td->tlflags & TLFLAGS_GC_SAFE) == 0)
 			continue;
 		else if (((td->attr.flags & PTHREAD_SCOPE_SYSTEM) != 0) &&
 		    ((td->kse->k_kcb->kcb_kmbx.km_flags & KMF_DONE) == 0)) {
@@ -2384,7 +2388,14 @@ _thr_alloc(struct pthread *curthread)
 	if ((thread == NULL) &&
 	    ((thread = malloc(sizeof(struct pthread))) != NULL)) {
 		bzero(thread, sizeof(struct pthread));
-		if ((thread->tcb = _tcb_ctor(thread, curthread == NULL)) == NULL) {
+		if (curthread) {
+			_pthread_mutex_lock(&_tcb_mutex);
+			thread->tcb = _tcb_ctor(thread, 0 /* not initial tls */);
+			_pthread_mutex_unlock(&_tcb_mutex);
+		} else {
+			thread->tcb = _tcb_ctor(thread, 1 /* initial tls */);
+		}
+		if (thread->tcb == NULL) {
 			free(thread);
 			thread = NULL;
 		} else {
@@ -2420,7 +2431,7 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
 		thread->name = NULL;
 	}
 	if ((curthread == NULL) || (free_thread_count >= MAX_CACHED_THREADS)) {
-		thr_destroy(thread);
+		thr_destroy(curthread, thread);
 	} else {
 		/* Add the thread to the free thread list. */
 		crit = _kse_critical_enter();
@@ -2433,14 +2444,20 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
 }
 
 static void
-thr_destroy(struct pthread *thread)
+thr_destroy(struct pthread *curthread, struct pthread *thread)
 {
 	int i;
 
 	for (i = 0; i < MAX_THR_LOCKLEVEL; i++)
 		_lockuser_destroy(&thread->lockusers[i]);
 	_lock_destroy(&thread->lock);
-	_tcb_dtor(thread->tcb);
+	if (curthread) {
+		_pthread_mutex_lock(&_tcb_mutex);
+		_tcb_dtor(thread->tcb);
+		_pthread_mutex_unlock(&_tcb_mutex);
+	} else {
+		_tcb_dtor(thread->tcb);
+	}
 	free(thread->siginfo);
 	free(thread);
 }
diff --git a/lib/libkse/thread/thr_private.h b/lib/libkse/thread/thr_private.h
index b2d1987..b681b3d 100644
--- a/lib/libkse/thread/thr_private.h
+++ b/lib/libkse/thread/thr_private.h
@@ -753,9 +753,13 @@ struct pthread {
 #define THR_FLAGS_IN_RUNQ	0x0004	/* in run queue using pqe link */
 #define	THR_FLAGS_EXITING	0x0008	/* thread is exiting */
 #define	THR_FLAGS_SUSPENDED	0x0010	/* thread is suspended */
-#define	THR_FLAGS_GC_SAFE	0x0020	/* thread safe for cleaning */
-#define	THR_FLAGS_IN_TDLIST	0x0040	/* thread in all thread list */
-#define	THR_FLAGS_IN_GCLIST	0x0080	/* thread in gc list */
+
+	/* Thread list flags; only set with thread list lock held. */
+#define	TLFLAGS_GC_SAFE		0x0001	/* thread safe for cleaning */
+#define	TLFLAGS_IN_TDLIST	0x0002	/* thread in all thread list */
+#define	TLFLAGS_IN_GCLIST	0x0004	/* thread in gc list */
+	int			tlflags;
+
 	/*
 	 * Base priority is the user setable and retrievable priority
 	 * of the thread.  It is only affected by explicit calls to
@@ -897,30 +901,30 @@ do {									\
  * the gc list.
  */
 #define	THR_LIST_ADD(thrd) do {					\
-	if (((thrd)->flags & THR_FLAGS_IN_TDLIST) == 0) {	\
+	if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) == 0) {	\
 		TAILQ_INSERT_HEAD(&_thread_list, thrd, tle);	\
 		_thr_hash_add(thrd);				\
-		(thrd)->flags |= THR_FLAGS_IN_TDLIST;		\
+		(thrd)->tlflags |= TLFLAGS_IN_TDLIST;		\
 	}							\
 } while (0)
 #define	THR_LIST_REMOVE(thrd) do {				\
-	if (((thrd)->flags & THR_FLAGS_IN_TDLIST) != 0) {	\
+	if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) != 0) {	\
 		TAILQ_REMOVE(&_thread_list, thrd, tle);		\
 		_thr_hash_remove(thrd);				\
-		(thrd)->flags &= ~THR_FLAGS_IN_TDLIST;		\
+		(thrd)->tlflags &= ~TLFLAGS_IN_TDLIST;		\
 	}							\
 } while (0)
 #define	THR_GCLIST_ADD(thrd) do {				\
-	if (((thrd)->flags & THR_FLAGS_IN_GCLIST) == 0) {	\
+	if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) == 0) {	\
 		TAILQ_INSERT_HEAD(&_thread_gc_list, thrd, gcle);\
-		(thrd)->flags |= THR_FLAGS_IN_GCLIST;		\
+		(thrd)->tlflags |= TLFLAGS_IN_GCLIST;		\
 		_gc_count++;					\
 	}							\
 } while (0)
 #define	THR_GCLIST_REMOVE(thrd) do {				\
-	if (((thrd)->flags & THR_FLAGS_IN_GCLIST) != 0) {	\
+	if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) != 0) {	\
 		TAILQ_REMOVE(&_thread_gc_list, thrd, gcle);	\
-		(thrd)->flags &= ~THR_FLAGS_IN_GCLIST;		\
+		(thrd)->tlflags &= ~TLFLAGS_IN_GCLIST;		\
 		_gc_count--;					\
 	}							\
 } while (0)
diff --git a/lib/libkse/thread/thr_sig.c b/lib/libkse/thread/thr_sig.c
index 81982d7..adca517 100644
--- a/lib/libkse/thread/thr_sig.c
+++ b/lib/libkse/thread/thr_sig.c
@@ -1199,8 +1199,7 @@ static void
 thr_sigframe_save(struct pthread *thread, struct pthread_sigframe *psf)
 {
 	/* This has to initialize all members of the sigframe. */
-	psf->psf_flags =
-	    thread->flags & (THR_FLAGS_PRIVATE | THR_FLAGS_IN_TDLIST);
+	psf->psf_flags = thread->flags & THR_FLAGS_PRIVATE;
 	psf->psf_interrupted = thread->interrupted;
 	psf->psf_timeout = thread->timeout;
 	psf->psf_state = thread->state;