Fix flaw in thread creation APIs

The arguments passed to the thread function must be allocated on
the heap, rather than the stack, since it is possible for the
spawning thread to continue before the new thread runs at all.
In such a case, it is possible that the area of stack where the
thread args were stored is overwritten.

* src/util/threads-pthread.c, src/util/threads-win32.c: Allocate
  thread arguments on the heap

diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index 02070ae3509065a0fe1595e2b3134a1fe3b35df1..bff49797bcb0fc0e8e0f595e766e3549488187db 100644 (file)
--- a/src/util/threads-pthread.c
+++ b/src/util/threads-pthread.c
@@ -26,6 +26,8 @@
 # include <sys/syscall.h>
 #endif
 
+#include "memory.h"
+
 
 /* Nothing special required for pthreads */
 int virThreadInitialize(void)
@@ -143,6 +145,7 @@ static void *virThreadHelper(void *data)
 {
     struct virThreadArgs *args = data;
     args->func(args->opaque);
+    VIR_FREE(args);
     return NULL;
 }
 
@@ -151,17 +154,25 @@ int virThreadCreate(virThreadPtr thread,
                     virThreadFunc func,
                     void *opaque)
 {
-    struct virThreadArgs args = { func, opaque };
+    struct virThreadArgs *args;
     pthread_attr_t attr;
     pthread_attr_init(&attr);
+    if (VIR_ALLOC(args) < 0)
+        return -1;
+
+    args->func = func;
+    args->opaque = opaque;
+
     if (!joinable)
         pthread_attr_setdetachstate(&attr, 1);
 
-    int ret = pthread_create(&thread->thread, &attr, virThreadHelper, &args);
+    int ret = pthread_create(&thread->thread, &attr, virThreadHelper, args);
     if (ret != 0) {
+        VIR_FREE(args);
         errno = ret;
         return -1;
     }
+    /* New thread owns 'args' in success case, so don't free */
     return 0;
 }
 

diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 33be4cf47739e6c4a1e35a661ba5f05acc23a1fb..436b3bd31ddc31414e186d63325b89698dc86940 100644 (file)
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -230,6 +230,8 @@ static void virThreadHelperDaemon(void *data)
 
     TlsSetValue(selfkey, NULL);
     CloseHandle(self.thread);
+
+    VIR_FREE(args);
 }
 
 static unsigned int __stdcall virThreadHelperJoinable(void *data)
@@ -249,6 +251,8 @@ static unsigned int __stdcall virThreadHelperJoinable(void *data)
 
     TlsSetValue(selfkey, NULL);
     CloseHandle(self.thread);
+
+    VIR_FREE(args);
     return 0;
 }
 
@@ -257,17 +261,24 @@ int virThreadCreate(virThreadPtr thread,
                     virThreadFunc func,
                     void *opaque)
 {
-    struct virThreadArgs args = { func, opaque };
+    struct virThreadArgs *args;
+
+    if (VIR_ALLOC(args) < 0)
+        return -1;
+
+    args->func = func;
+    args->opaque = opaque;
+
     thread->joinable = joinable;
     if (joinable) {
         thread->thread = (HANDLE)_beginthreadex(NULL, 0,
                                                 virThreadHelperJoinable,
-                                                &args, 0, NULL);
+                                                args, 0, NULL);
         if (thread->thread == 0)
             return -1;
     } else {
         thread->thread = (HANDLE)_beginthread(virThreadHelperDaemon,
-                                              0, &args);
+                                              0, args);
         if (thread->thread == (HANDLE)-1L)
             return -1;
     }