[PATCH] Fix flaw in thread creation APIs

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

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
---
 src/util/threads-pthread.c |   15 +++++++++++++--
 src/util/threads-win32.c   |   17 ++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index 02070ae..bff4979 100644
--- 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 33be4cf..436b3bd 100644
--- 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;
     }
-- 
1.7.2.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list