From 9c6560924721de358b240c43bed068f97b0e2364 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Wed, 1 Dec 2010 11:23:07 +0000
Subject: [PATCH] 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
---
 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 02070ae350..bff49797bc 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 33be4cf477..436b3bd31d 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;
     }
-- 
GitLab