From 16ede4bcaff474bba3f514bafc8d4a46390c2bdb Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Sun, 24 Sep 2023 14:21:30 -0400 Subject: [PATCH 1/8] remove usage of deprecated ATOMIC_VAR_INIT --- include/eosio/vm/profile.hpp | 2 +- include/eosio/vm/signals.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/eosio/vm/profile.hpp b/include/eosio/vm/profile.hpp index dc83b0e4..92676d36 100644 --- a/include/eosio/vm/profile.hpp +++ b/include/eosio/vm/profile.hpp @@ -341,7 +341,7 @@ struct scoped_profile { #else __attribute__((visibility("default"))) -inline thread_local std::atomic per_thread_profile_data = ATOMIC_VAR_INIT(nullptr); +inline thread_local std::atomic per_thread_profile_data{nullptr}; inline void profile_handler(int sig, siginfo_t* info, void* uc) { static_assert(std::atomic::is_always_lock_free); diff --git a/include/eosio/vm/signals.hpp b/include/eosio/vm/signals.hpp index 3de07498..05bbdab8 100644 --- a/include/eosio/vm/signals.hpp +++ b/include/eosio/vm/signals.hpp @@ -14,7 +14,7 @@ namespace eosio { namespace vm { // Fixes a duplicate symbol build issue when building with `-fvisibility=hidden` __attribute__((visibility("default"))) - inline thread_local std::atomic signal_dest = ATOMIC_VAR_INIT(nullptr); + inline thread_local std::atomic signal_dest{nullptr}; // Fixes a duplicate symbol build issue when building with `-fvisibility=hidden` __attribute__((visibility("default"))) From 407660b213c944aa2aed85e09f8485e6c9170aa7 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 7 Mar 2024 21:59:58 -0500 Subject: [PATCH 2/8] guard against calling memcpy() with NULL src --- include/eosio/vm/execution_context.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/eosio/vm/execution_context.hpp b/include/eosio/vm/execution_context.hpp index 7bdf8d13..f0f33b8b 100644 --- a/include/eosio/vm/execution_context.hpp +++ b/include/eosio/vm/execution_context.hpp @@ -171,7 +171,8 @@ namespace eosio { namespace vm { auto required_memory = static_cast(offset) + data_seg.data.size(); EOS_VM_ASSERT(required_memory <= available_memory, wasm_memory_exception, "data out of range"); auto addr = _linear_memory + offset; - memcpy((char*)(addr), data_seg.data.data(), data_seg.data.size()); + if(data_seg.data.size()) + memcpy((char*)(addr), data_seg.data.data(), data_seg.data.size()); } // Globals can be different from one WASM code to another. From 5996e485e75b19788c448145f8c1f37b77a536b7 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 18 Mar 2024 16:30:29 -0400 Subject: [PATCH 3/8] only handle signals from expected memory ranges --- include/eosio/vm/allocator.hpp | 4 ++++ include/eosio/vm/execution_context.hpp | 6 +++--- include/eosio/vm/signals.hpp | 25 ++++++++++++++++++++++--- tests/signals_tests.cpp | 8 ++++++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/include/eosio/vm/allocator.hpp b/include/eosio/vm/allocator.hpp index 6ed2669a..c267ea7e 100644 --- a/include/eosio/vm/allocator.hpp +++ b/include/eosio/vm/allocator.hpp @@ -372,6 +372,8 @@ namespace eosio { namespace vm { const void* get_code_start() const { return _code_base; } + std::span get_code_span() const {return {(std::byte*)_code_base, _code_size};} + /* different semantics than free, * the memory must be at the end of the most recently allocated block. */ @@ -524,5 +526,7 @@ namespace eosio { namespace vm { inline T* create_pointer(uint32_t offset) { return reinterpret_cast(raw + offset); } inline int32_t get_current_page() const { return page; } bool is_in_region(char* p) { return p >= raw && p < raw + max_memory; } + + std::span get_span() const {return {(std::byte*)raw, max_memory};} }; }} // namespace eosio::vm diff --git a/include/eosio/vm/execution_context.hpp b/include/eosio/vm/execution_context.hpp index f0f33b8b..e6ac59ce 100644 --- a/include/eosio/vm/execution_context.hpp +++ b/include/eosio/vm/execution_context.hpp @@ -358,11 +358,11 @@ namespace eosio { namespace vm { vm::invoke_with_signal_handler([&]() { result = execute(args_raw, fn, this, base_type::linear_memory(), stack); - }, &handle_signal); + }, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()}); } else { vm::invoke_with_signal_handler([&]() { result = execute(args_raw, fn, this, base_type::linear_memory(), stack); - }, &handle_signal); + }, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()}); } } } catch(wasm_exit_exception&) { @@ -800,7 +800,7 @@ namespace eosio { namespace vm { setup_locals(func_index); vm::invoke_with_signal_handler([&]() { execute(visitor); - }, &handle_signal); + }, &handle_signal, {_mod->allocator.get_code_span(), base_type::get_wasm_allocator()->get_span()}); } if (_mod->get_function_type(func_index).return_count && !_state.exiting) { diff --git a/include/eosio/vm/signals.hpp b/include/eosio/vm/signals.hpp index 05bbdab8..f52bfe82 100644 --- a/include/eosio/vm/signals.hpp +++ b/include/eosio/vm/signals.hpp @@ -16,6 +16,9 @@ namespace eosio { namespace vm { __attribute__((visibility("default"))) inline thread_local std::atomic signal_dest{nullptr}; + __attribute__((visibility("default"))) + inline thread_local std::vector> protected_memory_ranges; + // Fixes a duplicate symbol build issue when building with `-fvisibility=hidden` __attribute__((visibility("default"))) inline thread_local std::exception_ptr saved_exception{nullptr}; @@ -25,7 +28,20 @@ namespace eosio { namespace vm { inline void signal_handler(int sig, siginfo_t* info, void* uap) { sigjmp_buf* dest = std::atomic_load(&signal_dest); - if (dest) { + + auto in_protected_range = [&]() { + //empty protection list means legacy catch-all behavior; useful for some of the old tests + if(protected_memory_ranges.empty()) + return true; + + for(const std::span& range : protected_memory_ranges) { + if(info->si_addr >= range.data() && info->si_addr < range.data() + range.size()) + return true; + } + return false; + }; + + if (dest && in_protected_range()) { siglongjmp(*dest, sig); } else { struct sigaction* prev_action; @@ -98,7 +114,9 @@ namespace eosio { namespace vm { sigaddset(&sa.sa_mask, SIGPROF); sa.sa_flags = SA_NODEFER | SA_SIGINFO; sigaction(SIGSEGV, &sa, &prev_signal_handler); +#ifndef __linux__ sigaction(SIGBUS, &sa, &prev_signal_handler); +#endif sigaction(SIGFPE, &sa, &prev_signal_handler); } @@ -114,16 +132,17 @@ namespace eosio { namespace vm { /// with non-trivial destructors, then it must mask the relevant signals /// during the lifetime of these objects or the behavior is undefined. /// - /// signals handled: SIGSEGV, SIGBUS, SIGFPE + /// signals handled: SIGSEGV, SIGBUS (except on Linux), SIGFPE /// // Make this noinline to prevent possible corruption of the caller's local variables. // It's unlikely, but I'm not sure that it can definitely be ruled out if both // this and f are inlined and f modifies locals from the caller. template - [[gnu::noinline]] auto invoke_with_signal_handler(F&& f, E&& e) { + [[gnu::noinline]] auto invoke_with_signal_handler(F&& f, E&& e, const std::vector>& protect_ranges) { setup_signal_handler(); sigjmp_buf dest; sigjmp_buf* volatile old_signal_handler = nullptr; + protected_memory_ranges = protect_ranges; int sig; if((sig = sigsetjmp(dest, 1)) == 0) { // Note: Cannot use RAII, as non-trivial destructors w/ longjmp diff --git a/tests/signals_tests.cpp b/tests/signals_tests.cpp index 15e5caf3..6dfba49f 100644 --- a/tests/signals_tests.cpp +++ b/tests/signals_tests.cpp @@ -15,7 +15,7 @@ TEST_CASE("Testing signals", "[invoke_with_signal_handler]") { std::raise(SIGSEGV); }, [](int sig) { throw test_exception{}; - }); + }, {}); } catch(test_exception&) { okay = true; } @@ -25,7 +25,7 @@ TEST_CASE("Testing signals", "[invoke_with_signal_handler]") { TEST_CASE("Testing throw", "[signal_handler_throw]") { CHECK_THROWS_AS(eosio::vm::invoke_with_signal_handler([](){ eosio::vm::throw_( "Exiting" ); - }, [](int){}), eosio::vm::wasm_exit_exception); + }, [](int){}, {}), eosio::vm::wasm_exit_exception); } static volatile sig_atomic_t sig_handled; @@ -54,9 +54,11 @@ TEST_CASE("Test signal handler forwarding", "[signal_handler_forward]") { sig_handled = 0; std::raise(SIGSEGV); CHECK(sig_handled == 42 + SIGSEGV); +#ifndef __linux__ sig_handled = 0; std::raise(SIGBUS); CHECK(sig_handled == 42 + SIGBUS); +#endif sig_handled = 0; std::raise(SIGFPE); CHECK(sig_handled == 42 + SIGFPE); @@ -73,9 +75,11 @@ TEST_CASE("Test signal handler forwarding", "[signal_handler_forward]") { sig_handled = 0; std::raise(SIGSEGV); CHECK(sig_handled == 142 + SIGSEGV); +#ifndef __linux__ sig_handled = 0; std::raise(SIGBUS); CHECK(sig_handled == 142 + SIGBUS); +#endif sig_handled = 0; std::raise(SIGFPE); CHECK(sig_handled == 142 + SIGFPE); From 9962c3728c66f4daa2fdc18193978b1316a6f905 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 18 Mar 2024 16:59:01 -0400 Subject: [PATCH 4/8] need include --- include/eosio/vm/signals.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/eosio/vm/signals.hpp b/include/eosio/vm/signals.hpp index f52bfe82..7387dea6 100644 --- a/include/eosio/vm/signals.hpp +++ b/include/eosio/vm/signals.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include From 4f9ef0e4ffea842e0f882a9828087efbad1ed105 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 18 Mar 2024 21:37:45 -0400 Subject: [PATCH 5/8] and another --- include/eosio/vm/allocator.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/eosio/vm/allocator.hpp b/include/eosio/vm/allocator.hpp index c267ea7e..1943990f 100644 --- a/include/eosio/vm/allocator.hpp +++ b/include/eosio/vm/allocator.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include From f725536d568a375103b063458f6e6d79233b522d Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 20 Mar 2024 15:44:22 -0400 Subject: [PATCH 6/8] replace lambda with function since we're going to longjmp out of this function, probably best to stay as trivial as possible --- include/eosio/vm/signals.hpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/include/eosio/vm/signals.hpp b/include/eosio/vm/signals.hpp index 7387dea6..b5a53127 100644 --- a/include/eosio/vm/signals.hpp +++ b/include/eosio/vm/signals.hpp @@ -27,22 +27,22 @@ namespace eosio { namespace vm { template inline struct sigaction prev_signal_handler; - inline void signal_handler(int sig, siginfo_t* info, void* uap) { - sigjmp_buf* dest = std::atomic_load(&signal_dest); + inline bool in_protected_range(void* addr) { + //empty protection list means legacy catch-all behavior; useful for some of the old tests + if(protected_memory_ranges.empty()) + return true; - auto in_protected_range = [&]() { - //empty protection list means legacy catch-all behavior; useful for some of the old tests - if(protected_memory_ranges.empty()) + for(const std::span& range : protected_memory_ranges) { + if(addr >= range.data() && addr < range.data() + range.size()) return true; + } + return false; + } - for(const std::span& range : protected_memory_ranges) { - if(info->si_addr >= range.data() && info->si_addr < range.data() + range.size()) - return true; - } - return false; - }; + inline void signal_handler(int sig, siginfo_t* info, void* uap) { + sigjmp_buf* dest = std::atomic_load(&signal_dest); - if (dest && in_protected_range()) { + if (dest && in_protected_range(info->si_addr)) { siglongjmp(*dest, sig); } else { struct sigaction* prev_action; From 76288f5f5d179f66e61d64e44452d4c636d281bd Mon Sep 17 00:00:00 2001 From: Steven Watanabe Date: Tue, 9 Apr 2024 13:02:55 -0600 Subject: [PATCH 7/8] Restore previous protected ranges --- include/eosio/vm/signals.hpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/eosio/vm/signals.hpp b/include/eosio/vm/signals.hpp index b5a53127..9f7d2baf 100644 --- a/include/eosio/vm/signals.hpp +++ b/include/eosio/vm/signals.hpp @@ -18,7 +18,7 @@ namespace eosio { namespace vm { inline thread_local std::atomic signal_dest{nullptr}; __attribute__((visibility("default"))) - inline thread_local std::vector> protected_memory_ranges; + inline thread_local std::atomic>*> protected_memory_ranges; // Fixes a duplicate symbol build issue when building with `-fvisibility=hidden` __attribute__((visibility("default"))) @@ -29,10 +29,11 @@ namespace eosio { namespace vm { inline bool in_protected_range(void* addr) { //empty protection list means legacy catch-all behavior; useful for some of the old tests - if(protected_memory_ranges.empty()) + const auto* ranges = protected_memory_ranges.load(); + if(!ranges || ranges->empty()) return true; - for(const std::span& range : protected_memory_ranges) { + for(const std::span& range : *ranges) { if(addr >= range.data() && addr < range.data() + range.size()) return true; } @@ -143,7 +144,7 @@ namespace eosio { namespace vm { setup_signal_handler(); sigjmp_buf dest; sigjmp_buf* volatile old_signal_handler = nullptr; - protected_memory_ranges = protect_ranges; + const auto old_protected_memory_ranges = protected_memory_ranges.exchange(&protect_ranges); int sig; if((sig = sigsetjmp(dest, 1)) == 0) { // Note: Cannot use RAII, as non-trivial destructors w/ longjmp @@ -165,13 +166,16 @@ namespace eosio { namespace vm { f(); pthread_sigmask(SIG_SETMASK, &old_sigmask, nullptr); std::atomic_store(&signal_dest, old_signal_handler); + std::atomic_store(&protected_memory_ranges, old_protected_memory_ranges); } catch(...) { pthread_sigmask(SIG_SETMASK, &old_sigmask, nullptr); std::atomic_store(&signal_dest, old_signal_handler); + std::atomic_store(&protected_memory_ranges, old_protected_memory_ranges); throw; } } else { std::atomic_store(&signal_dest, old_signal_handler); + std::atomic_store(&protected_memory_ranges, old_protected_memory_ranges); if (sig == -1) { std::exception_ptr exception = std::move(saved_exception); saved_exception = nullptr; From fa7157138a6d6da16c83ebc24e0895fe45f63bd8 Mon Sep 17 00:00:00 2001 From: Steven Watanabe Date: Tue, 9 Apr 2024 13:31:00 -0600 Subject: [PATCH 8/8] Remove more instances of memcpy with nullptr. --- include/eosio/vm/execution_context.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/eosio/vm/execution_context.hpp b/include/eosio/vm/execution_context.hpp index 2af26e8e..744cb82b 100644 --- a/include/eosio/vm/execution_context.hpp +++ b/include/eosio/vm/execution_context.hpp @@ -249,7 +249,8 @@ namespace eosio { namespace vm { } else { uint32_t offset = elem_seg.offset.value.i32; EOS_VM_ASSERT(static_cast(offset) + elem_seg.elems.size() <= mod.tables[0].limits.initial, wasm_memory_exception, "elem out of range"); - std::memcpy(table_start + offset, elem_seg.elems.data(), elem_seg.elems.size() * sizeof(table_entry)); + if (elem_seg.elems.size()) + std::memcpy(table_start + offset, elem_seg.elems.data(), elem_seg.elems.size() * sizeof(table_entry)); _dropped_elems[i] = true; } } @@ -264,7 +265,8 @@ namespace eosio { namespace vm { if (std::uint64_t{s} + n > data_len) throw_("data out of range"); void* dest = get_interface().template validate_pointer(d, n); - std::memcpy(dest, data_seg.data.data() + s, n); + if (data_len) + std::memcpy(dest, data_seg.data.data() + s, n); } void drop_data(uint32_t x) { @@ -292,7 +294,8 @@ namespace eosio { namespace vm { throw_("elem out of range"); if (std::uint64_t{d} + n > mod.tables[0].limits.initial) throw_("wasm memory out-of-bounds"); - std::memcpy(get_table_base() + d, elem_seg.elems.data() + s, n * sizeof(table_entry)); + if (elem_len) + std::memcpy(get_table_base() + d, elem_seg.elems.data() + s, n * sizeof(table_entry)); } void drop_elem(uint32_t x) {