From 4d5be5b7c1e4369d430610e57877411caedec564 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Thu, 29 Feb 2024 11:29:01 +0100 Subject: [PATCH] Make ddtrace_disable a true global to avoid crashes in ZTS with unsupported SAPI/extensions Signed-off-by: Bob Weinand --- ext/compatibility.h | 10 +++++++++ ext/ddtrace.c | 54 ++++++++++++++++++++++++++------------------- ext/ddtrace.h | 1 - 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/ext/compatibility.h b/ext/compatibility.h index 16f35a02a2..16ca4cee04 100644 --- a/ext/compatibility.h +++ b/ext/compatibility.h @@ -183,6 +183,16 @@ static inline HashTable *zend_new_array(uint32_t nSize) { #define zend_hash_str_add_new(...) _zend_hash_str_add_new(__VA_ARGS__ ZEND_FILE_LINE_CC) #define smart_str_free_ex(str, persistent) smart_str_free(str) + +static inline zend_bool zend_ini_parse_bool(zend_string *str) { + if ((ZSTR_LEN(str) == 4 && strcasecmp(ZSTR_VAL(str), "true") == 0) + || (ZSTR_LEN(str) == 3 && strcasecmp(ZSTR_VAL(str), "yes") == 0) + || (ZSTR_LEN(str) == 2 && strcasecmp(ZSTR_VAL(str), "on") == 0)) { + return 1; + } else { + return atoi(ZSTR_VAL(str)) != 0; + } +} #endif #if PHP_VERSION_ID < 70400 diff --git a/ext/ddtrace.c b/ext/ddtrace.c index f35dc30499..2915135b4d 100644 --- a/ext/ddtrace.c +++ b/ext/ddtrace.c @@ -129,13 +129,21 @@ TSRM_TLS void *TSRMLS_CACHE = NULL; #endif #endif +int ddtrace_disable = 0; // 0 = enabled, 1 = disabled via INI, 2 = disabled, but MINIT was fully executed +static ZEND_INI_MH(dd_OnUpdateDisabled) { + UNUSED(entry, mh_arg1, mh_arg2, mh_arg3, stage); + if (!ddtrace_disable) { + ddtrace_disable = zend_ini_parse_bool(new_value); + } + return SUCCESS; +} + PHP_INI_BEGIN() -STD_PHP_INI_BOOLEAN("ddtrace.disable", "0", PHP_INI_SYSTEM, OnUpdateBool, disable, zend_ddtrace_globals, - ddtrace_globals) + ZEND_INI_ENTRY("ddtrace.disable", "0", PHP_INI_SYSTEM, dd_OnUpdateDisabled) -// Exposed for testing only -STD_PHP_INI_ENTRY("ddtrace.cgroup_file", "/proc/self/cgroup", PHP_INI_SYSTEM, OnUpdateString, cgroup_file, - zend_ddtrace_globals, ddtrace_globals) + // Exposed for testing only + STD_PHP_INI_ENTRY("ddtrace.cgroup_file", "/proc/self/cgroup", PHP_INI_SYSTEM, OnUpdateString, cgroup_file, + zend_ddtrace_globals, ddtrace_globals) PHP_INI_END() #if PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400 @@ -389,7 +397,7 @@ static void dd_activate_once(void) { ddtrace_generate_runtime_id(); // must run before the first zai_hook_activate as ddtrace_telemetry_setup installs a global hook - if (!DDTRACE_G(disable)) { + if (!ddtrace_disable) { #ifndef _WIN32 if (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER()) #endif @@ -413,15 +421,15 @@ static void ddtrace_activate(void) { zend_hash_init(&DDTRACE_G(traced_spans), 8, unused, NULL, 0); zend_hash_init(&DDTRACE_G(tracestate_unknown_dd_keys), 8, unused, NULL, 0); - if (!DDTRACE_G(disable) && ddtrace_has_excluded_module == true) { - DDTRACE_G(disable) = 2; + if (!ddtrace_disable && ddtrace_has_excluded_module == true) { + ddtrace_disable = 2; } // ZAI config is always set up pthread_once(&dd_activate_once_control, dd_activate_once); zai_config_rinit(); - if (!DDTRACE_G(disable) && (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER())) { + if (!ddtrace_disable && (get_global_DD_INSTRUMENTATION_TELEMETRY_ENABLED() || get_global_DD_TRACE_SIDECAR_TRACE_SENDER())) { ddtrace_sidecar_ensure_active(); } @@ -430,11 +438,11 @@ static void ddtrace_activate(void) { dd_save_sampling_rules_file_config(sampling_rules_file, PHP_INI_USER, PHP_INI_STAGE_RUNTIME); } - if (!DDTRACE_G(disable) && strcmp(sapi_module.name, "cli") == 0 && !get_DD_TRACE_CLI_ENABLED()) { - DDTRACE_G(disable) = 2; + if (!ddtrace_disable && strcmp(sapi_module.name, "cli") == 0 && !get_DD_TRACE_CLI_ENABLED()) { + ddtrace_disable = 2; } - if (DDTRACE_G(disable)) { + if (ddtrace_disable) { ddtrace_disable_tracing_in_current_request(); } @@ -1002,7 +1010,7 @@ static void dd_disable_if_incompatible_sapi_detected(void) { datadog_php_string_view module_name = datadog_php_string_view_from_cstr(sapi_module.name); if (UNEXPECTED(!dd_is_compatible_sapi(module_name))) { LOG(WARN, "Incompatible SAPI detected '%s'; disabling ddtrace", sapi_module.name); - DDTRACE_G(disable) = 1; + ddtrace_disable = 1; } } @@ -1076,7 +1084,7 @@ static PHP_MINIT_FUNCTION(ddtrace) { mod_ptr->handle = NULL; /* }}} */ - if (DDTRACE_G(disable)) { + if (ddtrace_disable) { return SUCCESS; } @@ -1123,7 +1131,7 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) { UNREGISTER_INI_ENTRIES(); - if (DDTRACE_G(disable) == 1) { + if (ddtrace_disable == 1) { zai_config_mshutdown(); zai_json_shutdown_bindings(); return SUCCESS; @@ -1257,7 +1265,7 @@ static PHP_RINIT_FUNCTION(ddtrace) { zai_interceptor_rinit(); #endif - if (!DDTRACE_G(disable)) { + if (!ddtrace_disable) { // With internal functions also being hookable, they must not be hooked before the CG(map_ptr_base) is zeroed zai_hook_activate(); DDTRACE_G(active_stack) = ddtrace_init_root_span_stack(); @@ -1376,11 +1384,11 @@ static PHP_RSHUTDOWN_FUNCTION(ddtrace) { if (get_DD_TRACE_ENABLED()) { dd_force_shutdown_tracing(); - } else if (!DDTRACE_G(disable)) { + } else if (!ddtrace_disable) { dd_shutdown_hooks_and_observer(); } - if (!DDTRACE_G(disable)) { + if (!ddtrace_disable) { OBJ_RELEASE(&DDTRACE_G(active_stack)->std); DDTRACE_G(active_stack) = NULL; } @@ -1427,7 +1435,7 @@ bool ddtrace_alter_dd_trace_disabled_config(zval *old_value, zval *new_value) { return true; } - if (DDTRACE_G(disable)) { + if (ddtrace_disable) { return Z_TYPE_P(new_value) == IS_FALSE; // no changing to enabled allowed if globally disabled } @@ -1437,7 +1445,7 @@ bool ddtrace_alter_dd_trace_disabled_config(zval *old_value, zval *new_value) { if (Z_TYPE_P(old_value) == IS_FALSE) { dd_initialize_request(); - } else if (!DDTRACE_G(disable)) { // if this is true, the request has not been initialized at all + } else if (!ddtrace_disable) { // if this is true, the request has not been initialized at all ddtrace_close_all_open_spans(false); // All remaining userland spans (and root span) dd_clean_globals(); } @@ -1525,12 +1533,12 @@ static PHP_MINFO_FUNCTION(ddtrace) { php_info_print_box_end(); php_info_print_table_start(); - php_info_print_table_row(2, "Datadog tracing support", DDTRACE_G(disable) ? "disabled" : "enabled"); + php_info_print_table_row(2, "Datadog tracing support", ddtrace_disable ? "disabled" : "enabled"); php_info_print_table_row(2, "Version", PHP_DDTRACE_VERSION); _dd_info_tracer_config(); php_info_print_table_end(); - if (!DDTRACE_G(disable)) { + if (!ddtrace_disable) { _dd_info_diagnostics_table(); } @@ -1723,7 +1731,7 @@ PHP_FUNCTION(dd_trace_reset) { LOG_LINE_ONCE(ERROR, "Unexpected parameters to dd_trace_reset"); } - if (DDTRACE_G(disable)) { + if (ddtrace_disable) { RETURN_BOOL(0); } diff --git a/ext/ddtrace.h b/ext/ddtrace.h index df83959989..efa257e3b4 100644 --- a/ext/ddtrace.h +++ b/ext/ddtrace.h @@ -70,7 +70,6 @@ typedef struct { // clang-format off ZEND_BEGIN_MODULE_GLOBALS(ddtrace) char *auto_prepend_file; - uint8_t disable; // 0 = enabled, 1 = disabled via INI, 2 = disabled, but MINIT was fully executed zend_bool request_init_hook_loaded; uint32_t traces_group_id;