[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH] tests/vhost-user-test: Fix potential use-after-free
From: |
David Gibson |
Subject: |
[Qemu-devel] [PATCH] tests/vhost-user-test: Fix potential use-after-free |
Date: |
Wed, 2 Dec 2015 13:46:37 +1100 |
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 <address@hidden>
---
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
- [Qemu-devel] [PATCH] tests/vhost-user-test: Fix potential use-after-free,
David Gibson <=