From ae8eb74675722b57ab66a51f1d6f4f250137bb23 Mon Sep 17 00:00:00 2001 From: Xi Wang Date: Sat, 27 Oct 2012 02:02:01 -0400 Subject: [PATCH] Fix a potential NULL pointer dereference in _init_thread(). The first NULL pointer check against `attr' suggests that `attr' can be NULL. Then later `attr' is directly dereferenced, suggesting the opposite. if (attr == NULL) { ... } else { ... } ... if (attr->stack_base == ...) { ... } The public API pthread_create(3) allows NULL, and interprets it as "default". Our implementation actually swaps in a pointer to the global default pthread_attr_t, so we don't need any NULL checks in _init_thread. (The other internal caller passes its own pthread_attr_t.) Change-Id: I0a4e79b83f5989249556a07eed1f2887e96c915e Signed-off-by: Xi Wang --- libc/bionic/pthread.c | 15 ++++++--------- libc/bionic/pthread_internal.h | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c index 7c22b45b2..ac7c64e34 100644 --- a/libc/bionic/pthread.c +++ b/libc/bionic/pthread.c @@ -211,18 +211,14 @@ void __thread_entry(int (*func)(void*), void *arg, void **tls) #include __LIBC_ABI_PRIVATE__ -int _init_thread(pthread_internal_t* thread, pid_t kernel_id, pthread_attr_t* attr, +int _init_thread(pthread_internal_t* thread, pid_t kernel_id, const pthread_attr_t* attr, void* stack_base, bool add_to_thread_list) { int error = 0; - if (attr == NULL) { - thread->attr = gDefaultPthreadAttr; - } else { - thread->attr = *attr; - } + thread->attr = *attr; thread->attr.stack_base = stack_base; - thread->kernel_id = kernel_id; + thread->kernel_id = kernel_id; // Make a note of whether the user supplied this stack (so we know whether or not to free it). if (attr->stack_base == stack_base) { @@ -234,7 +230,8 @@ int _init_thread(pthread_internal_t* thread, pid_t kernel_id, pthread_attr_t* at struct sched_param param; param.sched_priority = thread->attr.sched_priority; if (sched_setscheduler(kernel_id, thread->attr.sched_policy, ¶m) == -1) { - // For back compat reasons, we just warn about possible invalid sched_policy + // For backwards compatibility reasons, we just warn about failures here. + // error = errno; const char* msg = "pthread_create sched_setscheduler call failed: %s\n"; __libc_android_log_print(ANDROID_LOG_WARN, "libc", msg, strerror(errno)); } @@ -361,7 +358,7 @@ int pthread_create(pthread_t *thread_out, pthread_attr_t const * attr, return clone_errno; } - int init_errno = _init_thread(thread, tid, (pthread_attr_t*) attr, stack, true); + int init_errno = _init_thread(thread, tid, attr, stack, true); if (init_errno != 0) { // Mark the thread detached and let its __thread_entry run to // completion. (It'll just exit immediately, cleaning up its resources.) diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 4bc81efe4..a6b44c718 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -54,7 +54,7 @@ typedef struct pthread_internal_t char dlerror_buffer[__BIONIC_DLERROR_BUFFER_SIZE]; } pthread_internal_t; -int _init_thread(pthread_internal_t* thread, pid_t kernel_id, pthread_attr_t* attr, +int _init_thread(pthread_internal_t* thread, pid_t kernel_id, const pthread_attr_t* attr, void* stack_base, bool add_to_thread_list); void _pthread_internal_add( pthread_internal_t* thread ); pthread_internal_t* __get_thread(void);