gumjs: Fix race in recv().wait() on application thread (#808)

frida · Jul 16, 2024 · 31f9661 · 31f9661
1 parent 235c7a5
commit 31f9661
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 3 deletions.
diff --git a/bindings/gumjs/gumquickcore.c b/bindings/gumjs/gumquickcore.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2020-2022 Francesco Tamagni <mrmacete@protonmail.ch>
  * Copyright (C) 2020 Marcus Mengs <mame8282@googlemail.com>
  * Copyright (C) 2021 Abdelrahman Eid <hot3eed@gmail.com>
+ * Copyright (C) 2024 Simon Zuckerbraun <Simon_Zuckerbraun@trendmicro.com>
  *
  * Licence: wxWindows Library Licence, Version 3.1
  */
@@ -2710,12 +2711,16 @@ GUMJS_DEFINE_FUNCTION (gumjs_set_incoming_message_callback)
 GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
 {
   GumQuickCore * self = core;
+  guint start_count;
   GumQuickScope scope = GUM_QUICK_SCOPE_INIT (self);
   GMainContext * context;
   gboolean called_from_js_thread;
-  guint start_count;
   gboolean event_source_available;
 
+  g_mutex_lock (&self->event_mutex);
+  start_count = self->event_count;
+  g_mutex_unlock (&self->event_mutex);
+
   _gum_quick_scope_perform_pending_io (self->current_scope);
 
   _gum_quick_scope_suspend (&scope);
@@ -2725,7 +2730,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
 
   g_mutex_lock (&self->event_mutex);
 
-  start_count = self->event_count;
   while (self->event_count == start_count && self->event_source_available)
   {
     if (called_from_js_thread)

diff --git a/bindings/gumjs/gumv8core.cpp b/bindings/gumjs/gumv8core.cpp
@@ -5,6 +5,7 @@
  * Copyright (C) 2020-2022 Francesco Tamagni <mrmacete@protonmail.ch>
  * Copyright (C) 2020 Marcus Mengs <mame8282@googlemail.com>
  * Copyright (C) 2021 Abdelrahman Eid <hot3eed@gmail.com>
+ * Copyright (C) 2024 Simon Zuckerbraun <Simon_Zuckerbraun@trendmicro.com>
  *
  * Licence: wxWindows Library Licence, Version 3.1
  */
@@ -1601,6 +1602,10 @@ GUMJS_DEFINE_FUNCTION (gumjs_set_incoming_message_callback)
 
 GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
 {
+  g_mutex_lock (&core->event_mutex);
+  auto start_count = core->event_count;
+  g_mutex_unlock (&core->event_mutex);
+
   gboolean event_source_available;
 
   core->current_scope->PerformPendingIO ();
@@ -1613,7 +1618,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
 
     g_mutex_lock (&core->event_mutex);
 
-    auto start_count = core->event_count;
     while (core->event_count == start_count && core->event_source_available)
     {
       if (called_from_js_thread)

diff --git a/tests/gumjs/script.c b/tests/gumjs/script.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2023 Grant Douglas <me@hexplo.it>
  * Copyright (C) 2024 Hillel Pinto <hillelpinto3@gmail.com>
  * Copyright (C) 2024 Håvard Sørbø <havard@hsorbo.no>
+ * Copyright (C) 2024 Simon Zuckerbraun <Simon_Zuckerbraun@trendmicro.com>
  *
  * Licence: wxWindows Library Licence, Version 3.1
  */
@@ -24,6 +25,7 @@ TESTLIST_BEGIN (script)
   TESTENTRY (recv_may_specify_desired_message_type)
   TESTENTRY (recv_can_be_waited_for_from_an_application_thread)
   TESTENTRY (recv_can_be_waited_for_from_two_application_threads)
+  TESTENTRY (recv_wait_in_an_application_thread_should_not_deadlock)
   TESTENTRY (recv_can_be_waited_for_from_our_js_thread)
   TESTENTRY (recv_wait_in_an_application_thread_should_throw_on_unload)
   TESTENTRY (recv_wait_in_our_js_thread_should_throw_on_unload)
@@ -6125,6 +6127,78 @@ TESTCASE (message_can_be_received)
   EXPECT_SEND_MESSAGE_WITH ("\"pong\"");
 }
 
+TESTCASE (recv_wait_in_an_application_thread_should_not_deadlock)
+{
+  GThread * worker_thread;
+  GumInvokeTargetContext ctx;
+  guint i;
+
+  if (!g_test_slow ())
+  {
+    g_print ("<skipping, run in slow mode> ");
+    return;
+  }
+
+  COMPILE_AND_LOAD_SCRIPT (
+      "Interceptor.replace(" GUM_PTR_CONST ", new NativeCallback(arg => {"
+      "  let timeToRecv;"
+      "  let shouldExit = false;"
+      "  while (true) {"
+      "    recv(message => {"
+      "       if (message.type === 'stop')"
+      "         shouldExit = true;"
+      "       else if (message.type === 'wait-until')"
+      "         timeToRecv = message.time;"
+      "       else"
+      "         throw new Error(`unexpected message: ${message.type}`);"
+      "    }).wait();"
+      "    if (shouldExit)"
+      "      return 0;"
+      "    while (Date.now() < timeToRecv) {}"
+      "    recv(message => {"
+      "      if (message.type !== 'ping')"
+      "        throw new Error(`unexpected message: ${message.type}`);"
+      "    }).wait();"
+      "    send('pong');"
+      "  }"
+      "}, 'int', ['int']));", target_function_int);
+
+  ctx.script = fixture->script;
+  ctx.repeat_duration = 0;
+  ctx.started = 0;
+  ctx.finished = 0;
+  worker_thread = g_thread_new ("script-test-worker-thread",
+      invoke_target_function_int_worker, &ctx);
+  while (ctx.started == 0)
+    g_usleep (G_USEC_PER_SEC / 200);
+
+  for (i = 0; i != 100; i++)
+  {
+    gint64 time_now, time_to_schedule_recv, time_to_post;
+    gchar * msg;
+
+    time_now = g_get_real_time ();
+    time_to_schedule_recv = time_now - (time_now % (20 * 1000)) + (50 * 1000);
+    time_to_post = time_to_schedule_recv + i;
+
+    msg = g_strdup_printf (
+        "{\"type\":\"wait-until\",\"time\":%" G_GINT64_FORMAT "}",
+        time_to_schedule_recv / 1000);
+    POST_MESSAGE (msg);
+    g_free (msg);
+
+    while (g_get_real_time () < time_to_post)
+      ;
+    POST_MESSAGE ("{\"type\":\"ping\"}");
+    EXPECT_SEND_MESSAGE_WITH ("\"pong\"");
+  }
+
+  POST_MESSAGE ("{\"type\":\"stop\"}");
+
+  g_thread_join (worker_thread);
+  g_assert_cmpint (ctx.finished, ==, 1);
+}
+
 TESTCASE (message_can_be_received_with_data)
 {
   const guint8 data_to_send[2] = { 0x13, 0x37 };