commit ae8eb74675722b57ab66a51f1d6f4f250137bb23
author Xi Wang <xi.wang@gmail.com> Sat Oct 27 02:02:01 2012 -0400
committer Elliott Hughes <enh@google.com> Mon Oct 29 08:22:13 2012 -0700
tree 7a5b7bf7eed38b5a7a5129cd1f261e66277381b0
parent 33c4a3adde3cf1753af0661f48ce86358e105277

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 <xi.wang@gmail.com>

libc/bionic/pthread.c

diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c
index 7c22b45..ac7c64e 100644
--- a/libc/bionic/pthread.c
+++ b/libc/bionic/pthread.c
@@ -211,18 +211,14 @@
 #include <private/logd.h>
 
 __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 @@
         struct sched_param param;
         param.sched_priority = thread->attr.sched_priority;
         if (sched_setscheduler(kernel_id, thread->attr.sched_policy, &param) == -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 @@
         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.)

libc/bionic/pthread_internal.h

diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index 4bc81ef..a6b44c7 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -54,7 +54,7 @@
     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);