From 5b18ca7d3f1a202d3e1bf580d90f7c2be4865491 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Mon, 31 Mar 2025 14:03:57 -0400 Subject: [PATCH] linux: Fix stack-related sub-process shutdown crashes (fixes #3912) On Linux systems the stack frame reference canary will be purposely changed when forking sub-processes (see https://crbug.com/40181003). To avoid sub-process shutdown crashes the NO_STACK_PROTECTOR annotation must be added to all functions in the call stack leading to CefExecuteProcess(). Applications that cannot add this annotation must instead pass the `--change-stack-guard-on-fork=disable` command-line flag. --- include/base/cef_compiler_specific.h | 24 ++++++++++++++++++++++++ include/cef_app.h | 2 +- libcef/browser/context.cc | 1 + libcef/browser/main_runner.cc | 1 + tests/cefclient/cefclient_gtk.cc | 2 ++ tests/cefsimple/cefsimple_linux.cc | 1 + tests/ceftests/run_all_unittests.cc | 1 + tools/cef_parser.py | 6 +++--- tools/make_cpptoc_impl.py | 7 +++++-- tools/make_ctocpp_impl.py | 2 ++ 10 files changed, 41 insertions(+), 6 deletions(-) diff --git a/include/base/cef_compiler_specific.h b/include/base/cef_compiler_specific.h index 24bcfb24b..e124dc425 100644 --- a/include/base/cef_compiler_specific.h +++ b/include/base/cef_compiler_specific.h @@ -313,6 +313,30 @@ #define STACK_UNINITIALIZED #endif +// Attribute "no_stack_protector" disables -fstack-protector for the specified +// function. +// +// "stack_protector" is enabled on most POSIX builds. The flag adds a canary +// to each stack frame, which on function return is checked against a reference +// canary. If the canaries do not match, it's likely that a stack buffer +// overflow has occurred, so immediately crashing will prevent exploitation in +// many cases. +// +// In some cases it's desirable to remove this, e.g. on hot functions, or if +// we have purposely changed the reference canary. +// +// On Linux systems the reference canary will be purposely changed when forking +// sub-processes (see https://crbug.com/40181003). To avoid sub-process shutdown +// crashes the NO_STACK_PROTECTOR annotation must be added to all functions in +// the call stack leading to CefExecuteProcess(). Applications that cannot add +// this annotation must instead pass the `--change-stack-guard-on-fork=disable` +// command-line flag. +#if defined(COMPILER_GCC) || defined(__clang__) +#define NO_STACK_PROTECTOR __attribute__((no_stack_protector)) +#else +#define NO_STACK_PROTECTOR +#endif + // The ANALYZER_ASSUME_TRUE(bool arg) macro adds compiler-specific hints // to Clang which control what code paths are statically analyzed, // and is meant to be used in conjunction with assert & assert-like functions. diff --git a/include/cef_app.h b/include/cef_app.h index 4d7a32ac5..5d6e4e9ea 100644 --- a/include/cef_app.h +++ b/include/cef_app.h @@ -59,7 +59,7 @@ class CefApp; /// |windows_sandbox_info| parameter is only used on Windows and may be NULL /// (see cef_sandbox_win.h for details). /// -/*--cef(api_hash_check,optional_param=application, +/*--cef(api_hash_check,no_stack_protector,optional_param=application, optional_param=windows_sandbox_info)--*/ int CefExecuteProcess(const CefMainArgs& args, CefRefPtr application, diff --git a/libcef/browser/context.cc b/libcef/browser/context.cc index fc1abef02..310c5e085 100644 --- a/libcef/browser/context.cc +++ b/libcef/browser/context.cc @@ -291,6 +291,7 @@ int RunMainWithPreferredStackSize(FiberState& fiber_state) { } // namespace +NO_STACK_PROTECTOR int CefExecuteProcess(const CefMainArgs& args, CefRefPtr application, void* windows_sandbox_info) { diff --git a/libcef/browser/main_runner.cc b/libcef/browser/main_runner.cc index b302f9edd..735f13115 100644 --- a/libcef/browser/main_runner.cc +++ b/libcef/browser/main_runner.cc @@ -206,6 +206,7 @@ void CefMainRunner::QuitMessageLoop() { } // static +NO_STACK_PROTECTOR int CefMainRunner::RunAsHelperProcess(const CefMainArgs& args, CefRefPtr application, void* windows_sandbox_info) { diff --git a/tests/cefclient/cefclient_gtk.cc b/tests/cefclient/cefclient_gtk.cc index 1bda0fd5b..e59b99564 100644 --- a/tests/cefclient/cefclient_gtk.cc +++ b/tests/cefclient/cefclient_gtk.cc @@ -48,6 +48,7 @@ void TerminationSignalHandler(int signatl) { MainContext::Get()->GetRootWindowManager()->CloseAllWindows(true); } +NO_STACK_PROTECTOR int RunMain(int argc, char* argv[]) { // Create a copy of |argv| on Linux because Chromium mangles the value // internally (see issue #620). @@ -158,6 +159,7 @@ int RunMain(int argc, char* argv[]) { } // namespace client // Program entry point function. +NO_STACK_PROTECTOR int main(int argc, char* argv[]) { return client::RunMain(argc, argv); } diff --git a/tests/cefsimple/cefsimple_linux.cc b/tests/cefsimple/cefsimple_linux.cc index 277cacb03..ce9de2239 100644 --- a/tests/cefsimple/cefsimple_linux.cc +++ b/tests/cefsimple/cefsimple_linux.cc @@ -31,6 +31,7 @@ int XIOErrorHandlerImpl(Display* display) { #endif // defined(CEF_X11) // Entry point function for all processes. +NO_STACK_PROTECTOR int main(int argc, char* argv[]) { // Provide CEF with command-line arguments. CefMainArgs main_args(argc, argv); diff --git a/tests/ceftests/run_all_unittests.cc b/tests/ceftests/run_all_unittests.cc index 26146d079..a74350c04 100644 --- a/tests/ceftests/run_all_unittests.cc +++ b/tests/ceftests/run_all_unittests.cc @@ -134,6 +134,7 @@ class ScopedPlatformSetup final { } // namespace +NO_STACK_PROTECTOR int main(int argc, char* argv[]) { int exit_code; diff --git a/tools/cef_parser.py b/tools/cef_parser.py index c8593df2d..f8d690aff 100644 --- a/tools/cef_parser.py +++ b/tools/cef_parser.py @@ -522,9 +522,9 @@ def dict_to_str(dict): # Attribute keys allowed in CEF metadata comments. COMMON_ATTRIB_KEYS = ('added', 'removed') CLASS_ATTRIB_KEYS = COMMON_ATTRIB_KEYS + ('no_debugct_check', 'source') -FUNCTION_ATTRIB_KEYS = COMMON_ATTRIB_KEYS + ('api_hash_check', 'capi_name', - 'count_func', 'default_retval', - 'index_param', 'optional_param') +FUNCTION_ATTRIB_KEYS = COMMON_ATTRIB_KEYS + ( + 'api_hash_check', 'capi_name', 'count_func', 'default_retval', + 'index_param', 'no_stack_protector', 'optional_param') # regex for matching comment-formatted attributes _cre_attrib = r'/\*--cef\(([A-Za-z0-9_ ,=:\n]{0,})\)--\*/' diff --git a/tools/make_cpptoc_impl.py b/tools/make_cpptoc_impl.py index f73a99025..2976d9603 100644 --- a/tools/make_cpptoc_impl.py +++ b/tools/make_cpptoc_impl.py @@ -9,10 +9,13 @@ import functools def make_cpptoc_impl_proto(name, func, parts): + proto = '' + if func.has_attrib('no_stack_protector'): + proto += 'NO_STACK_PROTECTOR ' if isinstance(func, obj_function_virtual): - proto = parts['retval'] + ' CEF_CALLBACK' + proto += parts['retval'] + ' CEF_CALLBACK' else: - proto = 'CEF_EXPORT ' + parts['retval'] + proto += 'CEF_EXPORT ' + parts['retval'] proto += ' ' + name + '(' + ', '.join(parts['args']) + ')' return proto diff --git a/tools/make_ctocpp_impl.py b/tools/make_ctocpp_impl.py index 96a927f26..c6b56b4f3 100644 --- a/tools/make_ctocpp_impl.py +++ b/tools/make_ctocpp_impl.py @@ -11,6 +11,8 @@ def make_ctocpp_impl_proto(clsname, name, func, parts): const = '' proto = 'NO_SANITIZE("cfi-icall") ' + if func.has_attrib('no_stack_protector'): + proto += 'NO_STACK_PROTECTOR ' if clsname is None: proto += 'CEF_GLOBAL ' + parts['retval'] + ' ' else: