From b5e370f900820dc00f0dbf9e65614e003dca9a4e Mon Sep 17 00:00:00 2001 From: Simon Zuckerbraun Date: Mon, 1 Apr 2024 16:09:56 -0500 Subject: [PATCH] Fix: preserved thread's system error over NativeCallback invocations Though JavaScript function wrapped as a NativeCallback allows both inspecting and altering the current thread's system error, it wasn't possible to leave the system error as-is. Unless replaced by a value unequal to the original, the system error was being forced to 0 upon return from the NativeCallback. Fixed and tests added. --- bindings/gumjs/gumquickcore.c | 5 +++ bindings/gumjs/gumv8core.cpp | 1 + bindings/gumjs/gumv8core.h | 17 ++++++++ bindings/gumjs/gumv8stalker.cpp | 17 -------- tests/gumjs/script.c | 75 +++++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/bindings/gumjs/gumquickcore.c b/bindings/gumjs/gumquickcore.c index 236c2d53aa..caa22f5575 100644 --- a/bindings/gumjs/gumquickcore.c +++ b/bindings/gumjs/gumquickcore.c @@ -4447,6 +4447,9 @@ gum_quick_native_callback_invoke (ffi_cif * cif, int argc, i; JSValue * argv; JSValue result; + gint saved_system_error; + + saved_system_error = gum_thread_get_system_error(); #if defined (HAVE_I386) && defined (_MSC_VER) return_address = GPOINTER_TO_SIZE (_ReturnAddress ()); @@ -4556,6 +4559,8 @@ gum_quick_native_callback_invoke (ffi_cif * cif, JS_FreeValue (ctx, self->wrapper); _gum_quick_scope_leave (&scope); + + gum_thread_set_system_error(saved_system_error); } GUMJS_DEFINE_FINALIZER (gumjs_callback_context_finalize) diff --git a/bindings/gumjs/gumv8core.cpp b/bindings/gumjs/gumv8core.cpp index 544bd9fa4e..b4a900c1cc 100644 --- a/bindings/gumjs/gumv8core.cpp +++ b/bindings/gumjs/gumv8core.cpp @@ -3434,6 +3434,7 @@ gum_v8_native_callback_invoke (ffi_cif * cif, void ** args, void * user_data) { + GumV8SystemErrorPreservationScope error_scope; guintptr return_address = 0; guintptr stack_pointer = 0; guintptr frame_pointer = 0; diff --git a/bindings/gumjs/gumv8core.h b/bindings/gumjs/gumv8core.h index 7196c3ebe7..6730fd14b2 100644 --- a/bindings/gumjs/gumv8core.h +++ b/bindings/gumjs/gumv8core.h @@ -183,4 +183,21 @@ G_GNUC_INTERNAL void _gum_v8_core_post (GumV8Core * self, const gchar * message, G_GNUC_INTERNAL void _gum_v8_core_push_job (GumV8Core * self, GumScriptJobFunc job_func, gpointer data, GDestroyNotify data_destroy); +class GumV8SystemErrorPreservationScope +{ +public: + GumV8SystemErrorPreservationScope() + : saved_error(gum_thread_get_system_error()) + { + } + + ~GumV8SystemErrorPreservationScope() + { + gum_thread_set_system_error(saved_error); + } + +private: + gint saved_error; +}; + #endif diff --git a/bindings/gumjs/gumv8stalker.cpp b/bindings/gumjs/gumv8stalker.cpp index 1f90f0eb0f..cb229fa643 100644 --- a/bindings/gumjs/gumv8stalker.cpp +++ b/bindings/gumjs/gumv8stalker.cpp @@ -70,23 +70,6 @@ struct GumV8CallProbe GumV8Stalker * module; }; -class GumV8SystemErrorPreservationScope -{ -public: - GumV8SystemErrorPreservationScope () - : saved_error (gum_thread_get_system_error ()) - { - } - - ~GumV8SystemErrorPreservationScope () - { - gum_thread_set_system_error (saved_error); - } - -private: - gint saved_error; -}; - static gboolean gum_v8_stalker_on_flush_timer_tick (GumV8Stalker * self); GUMJS_DECLARE_GETTER (gumjs_stalker_get_trust_threshold) diff --git a/tests/gumjs/script.c b/tests/gumjs/script.c index e9588e5c40..c378aca3e7 100644 --- a/tests/gumjs/script.c +++ b/tests/gumjs/script.c @@ -67,6 +67,8 @@ TESTLIST_BEGIN (script) TESTENTRY (system_error_can_be_read_from_replacement_function) TESTENTRY (system_error_can_be_replaced_from_interceptor_listener) TESTENTRY (system_error_can_be_replaced_from_replacement_function) + TESTENTRY (system_error_unaffected_by_replacement_function_if_assigned_original_value) + TESTENTRY (system_error_unaffected_by_replacement_function_if_untouched) TESTENTRY (invocations_are_bound_on_tls_object) TESTENTRY (invocations_provide_thread_id) TESTENTRY (invocations_provide_call_depth) @@ -368,6 +370,7 @@ TESTLIST_BEGIN (script) TESTENTRY (cmodule_should_support_global_callbacks) TESTENTRY (cmodule_should_provide_access_to_cpu_registers) TESTENTRY (cmodule_should_provide_access_to_system_error) + TESTENTRY (system_error_unaffected_by_native_callback_from_cmodule) #else TESTENTRY (cmodule_constructor_should_throw_not_available) #endif @@ -6988,6 +6991,51 @@ TESTCASE (system_error_can_be_replaced_from_replacement_function) #endif } +TESTCASE(system_error_unaffected_by_replacement_function_if_assigned_original_value) +{ +#ifdef HAVE_WINDOWS + COMPILE_AND_LOAD_SCRIPT( + "Interceptor.replace(" GUM_PTR_CONST "," + " new NativeCallback(function (arg) {" + " this.lastError = 1337;" + " return 0;" + "}, 'int', ['int']));", target_function_int); + + SetLastError(1337); + target_function_int(7); + g_assert_cmpint(GetLastError(), == , 1337); +#else + COMPILE_AND_LOAD_SCRIPT( + "Interceptor.replace(" GUM_PTR_CONST "," + " new NativeCallback(function (arg) {" + " this.errno = 1337;" + " return 0;" + "}, 'int', ['int']));", target_function_int); + + errno = 1337; + target_function_int(7); + g_assert_cmpint(errno, == , 1337); +#endif +} + +TESTCASE(system_error_unaffected_by_replacement_function_if_untouched) +{ + COMPILE_AND_LOAD_SCRIPT( + "Interceptor.replace(" GUM_PTR_CONST "," + " new NativeCallback(function (arg) {" + " return 0;" + "}, 'int', ['int']));", target_function_int); +#ifdef HAVE_WINDOWS + SetLastError(1337); + target_function_int(7); + g_assert_cmpint(GetLastError(), == , 1337); +#else + errno = 1337; + target_function_int(7); + g_assert_cmpint(errno, == , 1337); +#endif +} + TESTCASE (invocations_are_bound_on_tls_object) { COMPILE_AND_LOAD_SCRIPT ( @@ -10195,6 +10243,33 @@ TESTCASE (cmodule_should_provide_access_to_system_error) g_assert_cmpint (gum_thread_get_system_error (), ==, 2); } +TESTCASE(system_error_unaffected_by_native_callback_from_cmodule) +{ + COMPILE_AND_LOAD_SCRIPT( + "const cm = new CModule(`\\n" + " #include \\n" + " extern void nativeCallback1();\\n" + "\\n" + " void replacement(GumInvocationContext * ic) {\\n" + " nativeCallback1(); }\\n" + "`,\n" + "{\n" + "'nativeCallback1' :\n" + " new NativeCallback(() => {}, 'void', [])\n" + "});\n" + "Interceptor.replace(" GUM_PTR_CONST ",\n" + " cm.replacement);", target_function_int); +#ifdef HAVE_WINDOWS + SetLastError(1337); + target_function_int(7); + g_assert_cmpint(GetLastError(), == , 1337); +#else + errno = 1337; + target_function_int(7); + g_assert_cmpint(errno, == , 1337); +#endif +} + #else /* !HAVE_TINYCC */ TESTCASE (cmodule_constructor_should_throw_not_available)