[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [nbdkit PATCH 4/4] sockets: Fix lifetime of thread_data



It is never a wise idea to pass stack-allocated storage to
another thread during pthread_create() unless you can guarantee
that the new thread will complete prior to the calling thread
returning and ending the lifetime of that storage.  We were
violating this, with the result in a SEGV in the detached child
thread during threadlocal_set_sockaddr() with parameters
pointing into thread_data which was reached at the same time
the main thread was trying to call exit().

Signed-off-by: Eric Blake <eblake redhat com>
---
 src/sockets.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/sockets.c b/src/sockets.c
index edb63b3..1d63523 100644
--- a/src/sockets.c
+++ b/src/sockets.c
@@ -261,6 +261,7 @@ start_thread (void *datav)

   handle_single_connection (data->sock, data->sock);

+  free (data);
   return NULL;
 }

@@ -270,18 +271,25 @@ accept_connection (int listen_sock)
   int err;
   pthread_attr_t attrs;
   pthread_t thread;
-  struct thread_data thread_data;
+  struct thread_data *thread_data;
   static size_t instance_num = 1;

-  thread_data.instance_num = instance_num++;
-  thread_data.addrlen = sizeof thread_data.addr;
+  thread_data = malloc (sizeof *thread_data);
+  if (!thread_data) {
+    perror ("malloc");
+    return;
+  }
+
+  thread_data->instance_num = instance_num++;
+  thread_data->addrlen = sizeof thread_data->addr;
  again:
-  thread_data.sock = accept (listen_sock,
-                             &thread_data.addr, &thread_data.addrlen);
-  if (thread_data.sock == -1) {
+  thread_data->sock = accept (listen_sock,
+                              &thread_data->addr, &thread_data->addrlen);
+  if (thread_data->sock == -1) {
     if (errno == EINTR || errno == EAGAIN)
       goto again;
     perror ("accept");
+    free (thread_data);
     return;
   }

@@ -291,16 +299,17 @@ accept_connection (int listen_sock)
    */
   pthread_attr_init (&attrs);
   pthread_attr_setdetachstate (&attrs, PTHREAD_CREATE_DETACHED);
-  err = pthread_create (&thread, &attrs, start_thread, &thread_data);
+  err = pthread_create (&thread, &attrs, start_thread, thread_data);
   pthread_attr_destroy (&attrs);
   if (err != 0) {
     fprintf (stderr, "%s: pthread_create: %s\n", program_name, strerror (err));
-    close (thread_data.sock);
+    close (thread_data->sock);
+    free (thread_data);
     return;
   }

   /* If the thread starts successfully, then it is responsible for
-   * closing the socket.
+   * closing the socket and freeing thread_data.
    */
 }

-- 
2.13.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]