diff --git a/.gitlab/generate-appsec.php b/.gitlab/generate-appsec.php index ce9113d68b..21bf7f8197 100644 --- a/.gitlab/generate-appsec.php +++ b/.gitlab/generate-appsec.php @@ -340,6 +340,9 @@ KUBERNETES_CPU_REQUEST: 8 KUBERNETES_MEMORY_REQUEST: 24Gi KUBERNETES_MEMORY_LIMIT: 30Gi + # Coverage instrumentation makes this build strictly bigger than the 30G + # its non-coverage sibling needs, and the DinD helper only defaults to 20G. + DOCKER_LOOPBACK_SIZE: 50G ARCH: amd64 GRADLE_USER_HOME: "$CI_PROJECT_DIR/.gradle-home" before_script: @@ -355,8 +358,21 @@ TERM=dumb ./gradlew loadCaches --info fi + echo "=== disk before buildPortableLibdatadogPhp ===" + df -h / + docker system df + docker run --rm -v php-portable-libdatadog-php:/vol alpine df -h /vol + + # Keep the exit status but always report disk after the build: an + # exhausted loopback is the hypothesis these numbers exist to settle. TERM=dumb ./gradlew buildPortableLibdatadogPhp \ - --info -Pbuildscan --scan -PuseHelperRustCoverage + --info -Pbuildscan --scan -PuseHelperRustCoverage && rc=0 || rc=$? + + echo "=== disk after buildPortableLibdatadogPhp (gradle exit $rc) ===" + df -h / + docker system df + docker run --rm -v php-portable-libdatadog-php:/vol alpine df -h /vol + [ "$rc" -eq 0 ] || exit "$rc" # Coverage-instrumented artifacts are bulky: this leaves ~6G of cargo # intermediates in php-portable-libdatadog-php, over a quarter of the diff --git a/appsec/src/extension/ddappsec.c b/appsec/src/extension/ddappsec.c index 672f6f7d41..273c9cb88e 100644 --- a/appsec/src/extension/ddappsec.c +++ b/appsec/src/extension/ddappsec.c @@ -158,6 +158,35 @@ static int ddappsec_startup(zend_extension *extension) return SUCCESS; } +// Native (not PHP) thread-local: the thread-exit destructor below may run after +// PHP has already torn this thread's globals down. +ZEND_TLS bool _tshutdown_ran; + +#if defined(ZTS) && defined(__linux__) +// Fallback for libcs without __cxa_thread_atexit_impl (musl): a pthread key +// whose destructor pthreads runs at thread exit. It takes no reference on this +// DSO -- harmless here, since musl never unmaps one, so the destructor pointer +// cannot outlive the image. +static pthread_key_t _tshutdown_key; +static int _tshutdown_key_err = -1; + +static void _create_tshutdown_key(void) +{ + _tshutdown_key_err = + pthread_key_create(&_tshutdown_key, _tshutdown_handler); +} + +static void _register_pthread_tshutdown_dtor(void) +{ + static pthread_once_t once_control = PTHREAD_ONCE_INIT; + pthread_once(&once_control, _create_tshutdown_key); + if (_tshutdown_key_err == 0) { + // Any non-NULL value will do; pthreads skips keys whose value is NULL. + pthread_setspecific(_tshutdown_key, (void *)(uintptr_t)1); + } +} +#endif + // GINIT/GSHUTDOWN run before/after MINIT/MSHUTDOWN static PHP_GINIT_FUNCTION(ddappsec) { @@ -170,6 +199,7 @@ static PHP_GINIT_FUNCTION(ddappsec) memset(ddappsec_globals, '\0', sizeof(*ddappsec_globals)); // NOLINT ddappsec_globals->to_be_configured = true; + _tshutdown_ran = false; // a thread may be reused for fresh globals #if ZTS // Record which thread these globals belong to so GSHUTDOWN can tell whether @@ -186,7 +216,7 @@ static PHP_GINIT_FUNCTION(ddappsec) // thread never exits, so the TLS destructor would pin ddappsec.so via // l_tls_dtor_count, which blocks DSO unload on apache graceful reload. // GSHUTDOWN calls _tshutdown_handler() directly for the main thread - // instead (see registered_thread_local_dtor below). + // instead (see GSHUTDOWN). # if PHP_VERSION_ID >= 70200 bool is_main_thread = tsrm_is_main_thread(); # else @@ -195,15 +225,19 @@ static PHP_GINIT_FUNCTION(ddappsec) if (!is_main_thread) { # if defined(__linux__) extern void *__dso_handle; - // adds a dependency on glibc 2.18 - extern int __cxa_thread_atexit_impl( - void (*func)(void *), void *arg, void *dso_handle); - __cxa_thread_atexit_impl(_tshutdown_handler, NULL, __dso_handle); + // Weak because musl exports no __cxa_thread_atexit_impl (it is a glibc + // 2.18 internal); a strong reference makes dlopen() of this DSO fail. + extern int __cxa_thread_atexit_impl(void (*func)(void *), void *arg, + void *dso_handle) __attribute__((weak)); + if (__cxa_thread_atexit_impl) { + __cxa_thread_atexit_impl(_tshutdown_handler, NULL, __dso_handle); + } else { + _register_pthread_tshutdown_dtor(); + } # elif defined(__APPLE__) extern void _tlv_atexit(void (*termFunc)(void *), void *objAddr); _tlv_atexit(_tshutdown_handler, NULL); # endif - ddappsec_globals->registered_thread_local_dtor = true; } #endif } @@ -211,6 +245,14 @@ static PHP_GINIT_FUNCTION(ddappsec) static void _tshutdown_handler(void *unspecnull ptr) { UNUSED(ptr); + // GSHUTDOWN and the thread-exit destructor can both target this thread; + // whichever runs first wins, as everything below is read from this thread's + // configuration, which the loser would find already freed. + if (_tshutdown_ran) { + return; + } + _tshutdown_ran = true; + mlog_g(dd_log_debug, "Running tshutdown (thread %" PRIxPTR ")", (uintptr_t)pthread_self()); @@ -228,14 +270,12 @@ static PHP_GSHUTDOWN_FUNCTION(ddappsec) #if ZTS // _tshutdown_handler() frees native thread-local storage, so it must run on - // the thread that owns it. Run it here only if (a) no thread-local - // destructor was registered to run it (main thread, or a platform without a - // thread-exit destructor mechanism) and (b) GSHUTDOWN is actually executing - // on the owning thread rather than on the main thread cleaning up an - // already exited thread's globals -- in which case its TLS is out of reach - // and there is nothing we can do. - if (!ddappsec_globals->registered_thread_local_dtor && - ddappsec_globals->ts_ls_cache == tsrm_get_ls_cache()) { + // the thread that owns it. Do it here whenever we are on that thread, which + // is also the only moment its configuration is guaranteed to still be + // mapped; the thread-local destructor registered in GINIT covers the other + // case, where we are the main thread cleaning up an already exited thread's + // globals and its TLS is out of reach. + if (ddappsec_globals->ts_ls_cache == tsrm_get_ls_cache()) { _tshutdown_handler(NULL); } #else diff --git a/appsec/src/extension/ddappsec.h b/appsec/src/extension/ddappsec.h index 6de8783334..bd3afb84ea 100644 --- a/appsec/src/extension/ddappsec.h +++ b/appsec/src/extension/ddappsec.h @@ -40,13 +40,6 @@ ZEND_BEGIN_MODULE_GLOBALS(ddappsec) // used to avoid a bailout during request shutdown bool during_request_shutdown : 1; -#ifdef ZTS - // true if a thread-local destructor was registered for this thread via - // __cxa_thread_atexit_impl() or _tlv_atexit(); when set, that destructor - // runs _tshutdown_handler() on thread exit, so GSHUTDOWN must not. - bool registered_thread_local_dtor : 1; -#endif - dd_conn conn; #ifdef ZTS