[Qemu-devel] [PATCH] tests/vhost-user-test: Fix potential use-after-free

[David Gibson]

ae31fb5 "vhost-user-test: wrap server in TestServer struct" cleaned up
the handling of the test server in vhost-user-test.  Unfortunately it
introduced a subtle use-after-free if a race goes the wrong way.

When the server structure is freed inside test_server_free() the GThread
started earlier is still running inside g_main_loop_run().  That GMainLoop
still has handlers active which reference the server structure, so if those
trip before the program exits there's a use-after-free.

I've had difficulty reproducing this locally, but for some reason it seems
to trip every time on Travis builds - this has been breaking all my test
builds there, which is why I notced it.

This patch prevents the use after free.  Unfortunately it looks like there
are additional problems still breaking my Travis builds, but one problem
at a time.

Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
---
 tests/vhost-user-test.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

NOTE: I'm not sure if fixing the race like this is the right approach.
It might be simpler just to remove the test_server_free() entirely,
letting the structure leak, since the program is about to terminate
anyway.

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index e4c36af..c547165 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -216,9 +216,10 @@ static void read_guest_mem(TestServer *s)
 
 static void *thread_function(void *data)
 {
-    GMainLoop *loop;
-    loop = g_main_loop_new(NULL, FALSE);
-    g_main_loop_run(loop);
+    GMainLoop **loopp = data;
+
+    *loopp = g_main_loop_new(NULL, FALSE);
+    g_main_loop_run(*loopp);
     return NULL;
 }
 
@@ -590,6 +591,8 @@ int main(int argc, char **argv)
     char *qemu_cmd = NULL;
     int ret;
     char template[] = "/tmp/vhost-test-XXXXXX";
+    GThread *thread;
+    GMainLoop *loop;
 
     g_test_init(&argc, &argv, NULL);
 
@@ -613,7 +616,7 @@ int main(int argc, char **argv)
     server = test_server_new("test");
 
     /* run the main loop thread so the chardev may operate */
-    g_thread_new(NULL, thread_function, NULL);
+    thread = g_thread_new(NULL, thread_function, &loop);
 
     qemu_cmd = GET_QEMU_CMD(server);
 
@@ -629,6 +632,9 @@ int main(int argc, char **argv)
         qtest_quit(s);
     }
 
+    g_main_loop_quit(loop);
+    g_thread_join(thread);
+
     /* cleanup */
     test_server_free(server);
 
-- 
2.5.0