bin/83457: [ PATCH ] Unhandled malloc failure within libpthread's _thr_alloc()

[Dan Lukes]

>Number:         83457
>Category:       bin
>Synopsis:       [ PATCH ] Unhandled malloc failure within libpthread's _thr_alloc()
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Jul 14 14:10:09 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dan Lukes
>Release:        FreeBSD 5.4-STABLE i386
>Organization:
Obludarium
>Environment:
System: FreeBSD kulesh.obluda.cz 5.4-STABLE FreeBSD 5.4-STABLE #8: Sat Jul 9 16:31:08 CEST 2005 dan at kulesh.obluda.cz:/usr/obj/usr/src/sys/KULESH i386
lib/libpthread/thread/thr_kern.c,v 1.112.2.2 2005/02/04 16:17:55 jhb

>Description:
	Unhandled malloc failure within libpthread's _thr_alloc()
>How-To-Repeat:
>Fix:

	The order of initialization of thread->tcb and thread->siginfo has
been swapped. It simplify cleanup after siginfo alloc failure (tcb is not
initialized yet, so it's not necesarry to destroy it)

	Over-nested if-else structures has been rearanged to improve
readability of code. Use 'goto' for exception handlings is not violation of
'structured programming rule'

--- patch begins here ---
--- lib/libpthread/thread/thr_kern.c.ORIG	Mon Feb 14 12:33:31 2005
+++ lib/libpthread/thread/thr_kern.c	Thu Jul 14 16:03:24 2005
@@ -2363,37 +2363,43 @@
 		}
 	}
 	if ((thread == NULL) &&
-	    ((thread = malloc(sizeof(struct pthread))) != NULL)) {
-		bzero(thread, sizeof(struct pthread));
-		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 {
-			thread->siginfo = calloc(_SIG_MAXSIG,
-				sizeof(siginfo_t));
-			/*
-			 * Initialize thread locking.
-			 * Lock initializing needs malloc, so don't
-			 * enter critical region before doing this!
-			 */
-			if (_lock_init(&thread->lock, LCK_ADAPTIVE,
-			    _thr_lock_wait, _thr_lock_wakeup) != 0)
-				PANIC("Cannot initialize thread lock");
-			for (i = 0; i < MAX_THR_LOCKLEVEL; i++) {
-				_lockuser_init(&thread->lockusers[i],
-				    (void *)thread);
-				_LCK_SET_PRIVATE2(&thread->lockusers[i],
-				    (void *)thread);
-			}
-		}
+	    ((thread = calloc(1, sizeof(struct pthread))) == NULL))
+		goto done;
+
+	thread->siginfo = calloc(_SIG_MAXSIG, sizeof(siginfo_t));
+	if (thread->siginfo == NULL) {
+		free(thread);
+		thread = NULL;
+		goto done;
+	}
+
+	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;
+		goto done;
+	}
+
+	/*
+	 * Initialize thread locking.
+	 * Lock initializing needs malloc, so don't
+	 * enter critical region before doing this!
+	 */
+	if (_lock_init(&thread->lock, LCK_ADAPTIVE,
+	    _thr_lock_wait, _thr_lock_wakeup) != 0)
+		PANIC("Cannot initialize thread lock");
+	for (i = 0; i < MAX_THR_LOCKLEVEL; i++) {
+		_lockuser_init(&thread->lockusers[i], (void *)thread);
+		_LCK_SET_PRIVATE2(&thread->lockusers[i], (void *)thread);
 	}
+
+done:
 	return (thread);
 }
 
@@ -2180,7 +2180,7 @@
 {
 	struct kse *kse = NULL;
 	char *stack;
-	kse_critical_t crit;
+	kse_critical_t crit = NULL;
 	int i;
 
 	if ((curthread != NULL) && (free_kse_count > 0)) {
--- patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted: