From 16003d50c177c79a9b8a5247cb31d220cdedb7da Mon Sep 17 00:00:00 2001 From: viferga Date: Thu, 25 Jul 2019 05:44:57 -0800 Subject: [PATCH] First base and research for ruby loader thread safety. --- .../loaders/rb_loader/source/rb_loader_impl.c | 117 ++++++++++++++++-- source/ports/node_port/test/index.js | 16 ++- 2 files changed, 118 insertions(+), 15 deletions(-) diff --git a/source/loaders/rb_loader/source/rb_loader_impl.c b/source/loaders/rb_loader/source/rb_loader_impl.c index 26d448f1b..3cb3e6072 100644 --- a/source/loaders/rb_loader/source/rb_loader_impl.c +++ b/source/loaders/rb_loader/source/rb_loader_impl.c @@ -31,9 +31,51 @@ #include +/* TODO: Make mprotect cross-platform */ +/* https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualprotect */ +/* http://pubs.opengroup.org/onlinepubs/009695399/functions/mprotect.html */ + +/* TODO: Secure stack */ +/* https://github.com/whitequark/coldruby/blob/master/libcoldruby/MRIRubyCompiler.cpp */ + +/* TODO: Bind stack */ +/* https://github.com/sunaku/ruby-coroutine-example/blob/master/main.c */ + +/* TODO: Protect against memory access from other threads */ +/* https://www.ruby-forum.com/t/ruby-embed-called-from-a-pthread/208641/6 */ +/* https://redmine.ruby-lang.org/issues/2294#note-18 */ +/* https://redmine.ruby-lang.org/issues/2294 */ + +#ifdef STACK_END_ADDRESS +#include /* TODO: Make this cross-platform */ +#endif + +#ifdef STACK_END_ADDRESS +extern void *STACK_END_ADDRESS; + +#define RUBY_PROLOGUE \ + do { \ + char stack_dummy;\ + do {\ + void *stack_backup = STACK_END_ADDRESS; \ + STACK_END_ADDRESS = &stack_dummy; + +#define RUBY_EPILOGUE \ + STACK_END_ADDRESS = stack_backup; \ + } while(0); \ + } while(0); +#else +#define RUBY_PROLOGUE +#define RUBY_EPILOGUE +#endif + #define LOADER_IMPL_RB_FUNCTION_ARGS_SIZE 0x10 #define LOADER_IMPL_RB_PROTECT_ARGS_SIZE 0x10 +#ifdef RUBY_GLOBAL_SETUP + RUBY_GLOBAL_SETUP +#endif + typedef struct loader_impl_rb_module_type { ID id; @@ -179,11 +221,15 @@ function_return function_rb_interface_invoke(function func, function_impl impl, VALUE result_value = Qnil; + value v = NULL; + if (args_size > LOADER_IMPL_RB_FUNCTION_ARGS_SIZE) { return NULL; } + RUBY_PROLOGUE + if (args_size > 0) { enum function_rb_interface_invoke_id @@ -317,16 +363,14 @@ function_return function_rb_interface_invoke(function func, function_impl impl, if (result_value != Qnil) { - value v = NULL; - const char * v_type_name = rb_type_deserialize(result_value, &v); signature_set_return(s, loader_impl_type(rb_function->impl, v_type_name)); - - return v; } - return NULL; + RUBY_EPILOGUE + + return v; } void function_rb_interface_destroy(function func, function_impl impl) @@ -404,11 +448,28 @@ loader_impl_data rb_loader_impl_initialize(loader_impl impl, configuration confi NULL }; + struct rb_loader_impl_type * rb_impl = &rb_loader_impl_unused; + +#ifdef STACK_END_ADDRESS + int page = sysconf(_SC_PAGE_SIZE); + mprotect((void *)((unsigned long)&STACK_END_ADDRESS & ~(page - 1)), page, PROT_READ | PROT_WRITE | PROT_EXEC); +#endif + (void)impl; (void)config; log_copy(host->log); + RUBY_PROLOGUE + + #ifdef HAVE_RUBY_SYSINIT + int argc = 0; + char * argv[] = { "" }; + ruby_sysinit(&argc, &argv); + #endif + + RUBY_INIT_STACK; + ruby_init(); ruby_init_loadpath(); @@ -417,23 +478,27 @@ loader_impl_data rb_loader_impl_initialize(loader_impl impl, configuration confi { ruby_cleanup(0); - return NULL; + rb_impl = NULL; } if (rb_gv_set("$VERBOSE", Qtrue) != Qtrue) { ruby_cleanup(0); - return NULL; + rb_impl = NULL; } + RUBY_EPILOGUE + log_write("metacall", LOG_LEVEL_DEBUG, "Ruby loader initialized correctly"); - return (loader_impl_data)&rb_loader_impl_unused; + return (loader_impl_data)rb_impl; } int rb_loader_impl_execution_path(loader_impl impl, const loader_naming_path path) { + RUBY_PROLOGUE + VALUE load_path_array = rb_gv_get("$:"); VALUE path_value = rb_str_new_cstr(path); @@ -442,6 +507,8 @@ int rb_loader_impl_execution_path(loader_impl impl, const loader_naming_path pat rb_ary_push(load_path_array, path_value); + RUBY_EPILOGUE + return 0; } @@ -643,6 +710,8 @@ loader_handle rb_loader_impl_load_from_file(loader_impl impl, const loader_namin return NULL; } + RUBY_PROLOGUE + for (iterator = 0; iterator < size; ++iterator) { loader_impl_rb_module rb_module; @@ -663,6 +732,8 @@ loader_handle rb_loader_impl_load_from_file(loader_impl impl, const loader_namin vector_push_back(handle->modules, &rb_module); } + RUBY_EPILOGUE + return (loader_handle)handle; } @@ -755,8 +826,12 @@ loader_handle rb_loader_impl_load_from_memory(loader_impl impl, const loader_nam return NULL; } + RUBY_PROLOGUE + rb_module = rb_loader_impl_load_from_memory_module(impl, name, buffer, size); + RUBY_EPILOGUE + if (rb_module == NULL) { log_write("metacall", LOG_LEVEL_ERROR, "Invalid ruby module loading from memory"); @@ -777,9 +852,13 @@ loader_handle rb_loader_impl_load_from_package(loader_impl impl, const loader_na { /* TODO */ + RUBY_PROLOGUE + (void)impl; (void)path; + RUBY_EPILOGUE + return NULL; } @@ -814,6 +893,8 @@ int rb_loader_impl_clear(loader_impl impl, loader_handle handle) size = vector_size(rb_handle->modules); + RUBY_PROLOGUE + for (iterator = 0; iterator < size; ++iterator) { loader_impl_rb_module * rb_module = vector_at(rb_handle->modules, iterator); @@ -833,6 +914,8 @@ int rb_loader_impl_clear(loader_impl impl, loader_handle handle) set_destroy((*rb_module)->function_map); } + RUBY_EPILOGUE + vector_destroy(rb_handle->modules); free(rb_handle); @@ -867,10 +950,14 @@ loader_impl_rb_function rb_function_create(loader_impl_rb_module rb_module, ID i if (rb_function != NULL) { + RUBY_PROLOGUE + rb_function->rb_module = rb_module; rb_function->method_id = id; rb_function->args_hash = rb_hash_new(); + RUBY_EPILOGUE + return rb_function; } @@ -945,6 +1032,8 @@ int rb_loader_impl_discover(loader_impl impl, loader_handle handle, context ctx) int result = 0; + RUBY_PROLOGUE + for (iterator = 0; iterator < size; ++iterator) { loader_impl_rb_module * rb_module = vector_at(rb_handle->modules, iterator); @@ -955,12 +1044,22 @@ int rb_loader_impl_discover(loader_impl impl, loader_handle handle, context ctx) } } + RUBY_EPILOGUE + return result; } int rb_loader_impl_destroy(loader_impl impl) { + int result; + (void)impl; - return ruby_cleanup(0); + RUBY_PROLOGUE + + result = ruby_cleanup(0); + + RUBY_EPILOGUE + + return result; } diff --git a/source/ports/node_port/test/index.js b/source/ports/node_port/test/index.js index a7608210e..04b351293 100644 --- a/source/ports/node_port/test/index.js +++ b/source/ports/node_port/test/index.js @@ -35,12 +35,16 @@ describe('metacall', () => { /* TODO: This creates a segmentation fault, it seems due to lack of thread-safety */ /* 54: -- C level backtrace information ------------------------------------------- - 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7fcfd8d27025] - 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7fcfd8d2725c] - 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7fcfd8c00904] - 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7fcfd8cb281e] - 54: /lib/x86_64-linux-gnu/libpthread.so.0 [0x7fcfdd60a0e0] - 54: [0x3089380fa9ff] + 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7f1c3b04a025] + 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7f1c3b04a25c] + 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7f1c3af23904] + 54: /usr/lib/x86_64-linux-gnu/libruby-2.3.so.2.3 [0x7f1c3afd581e] + 54: /lib/x86_64-linux-gnu/libpthread.so.0 [0x7f1c4b92e0e0] + 54: /usr/bin/node [0x8d5ea0] + 54: /usr/bin/node [0x8d660c] + 54: /usr/bin/node(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0x193) [0xa94a43] + 54: /usr/bin/node [0xb0bbec] + 54: /usr/bin/node(_ZN2v88internal21Builtin_HandleApiCallEiPPNS0_6ObjectEPNS0_7IsolateE+0xaf) [0xb0c83f] */ /* assert.strictEqual(metacall('get_second', 5, 12), 12); */ });