Skip to content

Commit

Permalink
prevent false-positive stack-buffer-overflow ASan detections on sig…
Browse files Browse the repository at this point in the history
…nal stack (#1201)

* Make signal stack unpoisoned
* Enable disabled OOM tests
---------

Signed-off-by: Petr Shumilov <p.shumilov@vkteam.ru>
  • Loading branch information
PetrShumilov authored Dec 27, 2024
1 parent 8664efb commit 11f593f
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
17 changes: 12 additions & 5 deletions server/php-runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,23 @@ void PhpScript::error(const char *error_message, script_error_t error_type, [[ma
stack_end = reinterpret_cast<char *>(get_context_stack_ptr_portable(exit_context)) + get_context_stack_size_portable(exit_context);

#if defined(__linux__) && defined(__x86_64__) || defined(__APPLE__) && (__aarch64__)
// The PhpScript::error may be produced in process of signal handling. The default behavior on Linux-based platforms
// consider to block a signal during handler execution and unblock after handler ending.
// For x86_64 arch we have context replacement implementation where the signals manipulations is omitted by design,
// e.g. we do not save signals state in context replacement.
// Therefore, we need manual control for signal state, especially, we have to unblock blocked signals at logical end of handler.
// The PhpScript::error function may be invoked during signal handling. The default behavior on Linux-based platforms
// is to block a signal during the execution of its handler and unblock it after the handler completes.
// For the x86_64 architecture, we have a context replacement implementation where signal manipulation is omitted by design;
// for example, we do not save the signal state during context replacement.
// Therefore, we need manual control over the signal state. Specifically, we must unblock any blocked signals at the logical end of the handler.
if (triggered_by_signal.has_value()) {
dl_unblock_signal(triggered_by_signal.value());
}
#endif
#if ASAN_ENABLED
// AddressSanitizer relies on normal function call and return patterns to maintain its internal stack of
// function calls, known as the "shadow stack," which helps it detect stack-related issues like "buffer overflows".
// Functions that do not return, e.g. using setcontext() functionality, can interfere with this, causing ASan to lose track of the actual state of the call stack.
// By calling __asan_handle_no_return(), we explicitly notify ASan that the current stack frame will not return
// as expected, allowing it to clean up and adjust its "shadow stack" correctly and avoid false-positive detections.
__asan_handle_no_return();

__sanitizer_start_switch_fiber(nullptr, main_thread_stack, main_thread_stacksize);
#endif
setcontext_portable(&exit_context);
Expand Down
3 changes: 0 additions & 3 deletions tests/python/tests/http_server/test_oom_handler.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import requests.exceptions
from python.lib.testcase import KphpServerAutoTestCase
import unittest


class TestOomHandler(KphpServerAutoTestCase):
Expand Down Expand Up @@ -47,12 +46,10 @@ def test_script_memory_realloc(self):
self._generic_test("test_case=script_memory_realloc")
self.kphp_server.assert_log(["realloc_allocations_cnt=1,realloc_mem_allocated=[1-9]\\d*"], timeout=5)

@unittest.skip("Temporary disabled until KPHP-1990 will be resolved")
def test_with_job_request(self):
self._generic_test("test_case=with_job_request")
self.kphp_server.assert_log(["job_request_succeeded=1"], timeout=5)

@unittest.skip("Temporary disabled until KPHP-1990 will be resolved")
def test_with_instance_cache(self):
self._generic_test("test_case=with_instance_cache")
self.kphp_server.assert_log(["instance_cache_store_succeeded=1"], timeout=5)
Expand Down

0 comments on commit 11f593f

Please sign in to comment.