diff --git a/.gitlab/build-loader.sh b/.gitlab/build-loader.sh index 7b8246d2332..5cc4a38bb00 100755 --- a/.gitlab/build-loader.sh +++ b/.gitlab/build-loader.sh @@ -22,4 +22,13 @@ phpize ./configure make clean make -j "${MAKE_JOBS}" all ECHO_ARG="-e" CFLAGS="-std=gnu11 -O2 -g -Wall -Wextra -Werror -DPHP_DD_LIBRARY_LOADER_VERSION='\"$(cat ../VERSION)\"'" + +# The reaper is copied out of the loader. Reject accidental references to its +# code/data, including compiler instrumentation, before packaging the library. +REAPER_RELOCATIONS="$(LC_ALL=C objdump -r -j ddloader_reaper_code .libs/telemetry_reaper.o)" +if [[ "${REAPER_RELOCATIONS}" == *"RELOCATION RECORDS"* ]]; then + printf 'Telemetry reaper must not contain relocations:\n%s\n' "${REAPER_RELOCATIONS}" + exit 1 +fi + cp modules/dd_library_loader.so "../dd_library_loader-$(uname -m)-${HOST_OS}.so" diff --git a/loader/config.m4 b/loader/config.m4 index 7618e40838d..3ca47796f94 100644 --- a/loader/config.m4 +++ b/loader/config.m4 @@ -8,5 +8,5 @@ if test "$PHP_DD_LIBRARY_LOADER" != "no"; then dnl In case of no dependencies AC_DEFINE(HAVE_DD_LIBRARY_LOADER, 1, [ Have dd_library_loader support ]) - PHP_NEW_EXTENSION(dd_library_loader, dd_library_loader.c dd_library_loader_module.c compat_php.c, $ext_shared, , , , yes) + PHP_NEW_EXTENSION(dd_library_loader, dd_library_loader.c dd_library_loader_module.c compat_php.c telemetry_reaper.c, $ext_shared, , , , yes) fi diff --git a/loader/dd_library_loader.c b/loader/dd_library_loader.c index e85aab1e1ff..0d5cd4797f9 100644 --- a/loader/dd_library_loader.c +++ b/loader/dd_library_loader.c @@ -9,13 +9,14 @@ #include #include #include -#include +#include #include #include
#include #include "compat_php.h" #include "php_dd_library_loader.h" +#include "telemetry_reaper.h" #define MIN_API_VERSION 320151012 #define MAX_API_VERSION 420250925 @@ -368,43 +369,9 @@ void ddloader_logf(injected_ext *config, log_level level, const char *format, .. va_end(va); } -typedef struct { - pid_t pid; - void *self_handle; // dlopen handle for this .so, closed by the thread -} ddloader_reaper_arg; - -// Reaps the telemetry child process, then tail-calls dlclose() to release the -// extra reference on this .so that was acquired before thread creation. -// The tail call ensures dlclose() returns directly to libpthread's start_thread -// without ever returning into this .so's code, which may be unmapped when -// dlclose() releases the last reference and runs munmap. -// -// [[clang::musttail]] guarantees the tail call at the source level (compile -// error if not possible). __attribute__((optimize("O2"))) is the GCC fallback -// to enable sibling-call optimisation. Both are needed because musttail -// requires a single CK_IntegralToPointer cast, while GCC -Wint-to-pointer-cast -// requires the (intptr_t) intermediate; using __has_attribute lets us pick the -// right form for each compiler. -// Redeclare dlclose under a private name with void* return type so the tail -// call is type-correct without any cast. int and void* share the same return -// register on all supported ABIs; the return value is discarded anyway. -extern void *ddloader_dlclose(void *) __asm__("dlclose"); - -#if defined(__has_attribute) && __has_attribute(musttail) -# define DDLOADER_MUSTTAIL __attribute__((musttail)) -#elif defined(__clang__) && __clang_major__ >= 13 -# define DDLOADER_MUSTTAIL [[clang::musttail]] -#else -# define DDLOADER_MUSTTAIL -__attribute__((optimize("O2"))) -#endif -static void *ddloader_reap_child(void *arg_) { - ddloader_reaper_arg *arg = (ddloader_reaper_arg *)arg_; - pid_t pid = arg->pid; - void *handle = arg->self_handle; - free(arg); - waitpid(pid, NULL, 0); - DDLOADER_MUSTTAIL return ddloader_dlclose(handle); +static void ddloader_wait_for_child(pid_t pid) { + while (waitpid(pid, NULL, 0) == -1 && errno == EINTR) { + } } /** @@ -501,22 +468,16 @@ static void ddloader_telemetryf(telemetry_reason reason, injected_ext *config, c return; } if (pid > 0) { - // reap the child in a background thread to avoid leaking it - ddloader_reaper_arg *reaper_arg = malloc(sizeof(*reaper_arg)); - reaper_arg->pid = pid; - // Bump our own refcount so this .so stays mapped while the reaper - // thread is running. The thread will tail-call dlclose() to release it. - Dl_info info; - reaper_arg->self_handle = - (dladdr((void *)ddloader_telemetryf, &info) && info.dli_fname) - ? dlopen(info.dli_fname, RTLD_LAZY) - : NULL; - pthread_t reaper; - pthread_attr_t attr; - pthread_attr_init(&attr); - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); - pthread_create(&reaper, &attr, ddloader_reap_child, reaper_arg); - pthread_attr_destroy(&attr); + // The reaper owns an independent code page, allowing Zend to unload + // this DSO during shutdown without waiting for telemetry delivery. + int error_code = ddloader_reaper_start(pid); + if (error_code) { + LOG(config, ERROR, "Telemetry error: cannot start child reaper: %s", strerror(error_code)) + // Do not let the forwarder block startup when no reaper thread is + // available. Reap it synchronously after forcing it to exit. + kill(pid, SIGKILL); + ddloader_wait_for_child(pid); + } return; // parent } @@ -609,7 +570,7 @@ static void ddloader_telemetryf(telemetry_reason reason, injected_ext *config, c // If execv failed, exit immediately // Return 127 for the most likely case of a missing file - exit(127); + _exit(127); } static char *ddloader_find_ext_path(const char *ext_dir, const char *ext_name, int module_api, bool is_zts, bool is_debug) { diff --git a/loader/telemetry_reaper.c b/loader/telemetry_reaper.c new file mode 100644 index 00000000000..b8e57506e7b --- /dev/null +++ b/loader/telemetry_reaper.c @@ -0,0 +1,122 @@ +#include "telemetry_reaper.h" + +#include +#include +#include +#include +#include +#include +#include + +#if (!defined(__x86_64__) && !defined(__aarch64__)) || defined(__ILP32__) +#error Unsupported architecture for the telemetry reaper +#endif + +#if defined(__aarch64__) && defined(__ARM_FEATURE_BTI_DEFAULT) +#include +#include + +// Compatibility with the CentOS 7 headers used for release builds. +#ifndef HWCAP2_BTI +#define HWCAP2_BTI (1UL << 17) +#endif +#ifndef PROT_BTI +#define PROT_BTI 0x10 +#endif +#endif + +// The context precedes the copied code; keep its entry point aligned. +typedef struct __attribute__((aligned(16))) { + size_t mapping_size; + pid_t pid; + pid_t (*wait_for_child)(pid_t, int *, int); + int *(*error_location)(void); + int (*unmap)(void *, size_t); +} ddloader_reaper_context; + +// Linker-provided bounds for the reaper code size. +extern const char __start_ddloader_reaper_code[] __attribute__((visibility("hidden"))); +extern const char __stop_ddloader_reaper_code[] __attribute__((visibility("hidden"))); + +#if __has_attribute(musttail) +#define DDLOADER_MUSTTAIL __attribute__((musttail)) +#elif defined(__clang__) && __clang_major__ >= 13 +#define DDLOADER_MUSTTAIL [[clang::musttail]] +#else +#define DDLOADER_MUSTTAIL +#endif + +// No code or data reference may point back into the loader, including compiler +// instrumentation. All libc calls go through pointers in the copied context. +// The release build checks that this section contains no relocations. +__attribute__((section("ddloader_reaper_code"), used, noinline, no_instrument_function, + no_profile_instrument_function)) +#if defined(__clang__) +// no_sanitize covers frontend checks; disabling instrumentation also removes +// TSan's function entry/exit hooks, which otherwise survive no_sanitize("all"). +__attribute__((no_stack_protector, no_sanitize("all"), disable_sanitizer_instrumentation)) +#else +// Older GCC has no musttail attribute; force sibling calls even in debug builds. +__attribute__((no_sanitize_address, no_sanitize_thread, no_sanitize_undefined, + optimize("O2", "optimize-sibling-calls", "no-stack-protector"))) +#endif +static int ddloader_reap_child(void *mapping, size_t unused) { + (void)unused; + ddloader_reaper_context *context = mapping; + while (context->wait_for_child(context->pid, NULL, 0) == -1 && + *context->error_location() == EINTR) { + } + // Match munmap's signature so musttail can guarantee that it returns straight + // to pthread's startup routine, never into the page it has just unmapped. + DDLOADER_MUSTTAIL return context->unmap(mapping, context->mapping_size); +} + +int ddloader_reaper_start(pid_t pid) { + size_t code_size = (uintptr_t)__stop_ddloader_reaper_code - (uintptr_t)__start_ddloader_reaper_code; + size_t entry_offset = (uintptr_t)ddloader_reap_child - (uintptr_t)__start_ddloader_reaper_code; + long page_size = sysconf(_SC_PAGESIZE); + if (page_size <= 0 || sizeof(ddloader_reaper_context) + code_size > (size_t)page_size) { + return EINVAL; + } + size_t mapping_size = (size_t)page_size; + ddloader_reaper_context *context = mmap(NULL, mapping_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (context == MAP_FAILED) { + return errno; + } + // Taking libc function addresses resolves them before this DSO can unload; + // the copied function must not use the loader's PLT or GOT, even for errno. + *context = (ddloader_reaper_context){mapping_size, pid, waitpid, __errno_location, munmap}; + void *code = context + 1; + memcpy(code, __start_ddloader_reaper_code, code_size); + __builtin___clear_cache(code, (char *)code + code_size); + + int protection = PROT_READ | PROT_EXEC; +#if defined(__aarch64__) && defined(__ARM_FEATURE_BTI_DEFAULT) + // Only enable BTI when the compiler emitted its landing pad in the copy. + if (getauxval(AT_HWCAP2) & HWCAP2_BTI) { + protection |= PROT_BTI; + } +#endif + if (mprotect(context, mapping_size, protection)) { + int error = errno; + munmap(context, mapping_size); + return error; + } + + pthread_attr_t attributes; + int error = pthread_attr_init(&attributes); + if (!error) { + error = pthread_attr_setdetachstate(&attributes, PTHREAD_CREATE_DETACHED); + if (!error) { + pthread_t thread; + // On the supported 64-bit ABIs the unused second argument needs no + // initialization, and a detached thread's return value is discarded. + error = pthread_create(&thread, &attributes, (void *(*)(void *))((char *)code + entry_offset), context); + } + pthread_attr_destroy(&attributes); + } + if (error) { + munmap(context, mapping_size); + } + return error; +} diff --git a/loader/telemetry_reaper.h b/loader/telemetry_reaper.h new file mode 100644 index 00000000000..a0996d06da9 --- /dev/null +++ b/loader/telemetry_reaper.h @@ -0,0 +1,10 @@ +#ifndef DDLOADER_TELEMETRY_REAPER_H +#define DDLOADER_TELEMETRY_REAPER_H + +#include + +// Start a detached reaper that can outlive the loader and frees its own code. +// Returns 0 on success, or an error number; on failure the caller must reap pid. +int ddloader_reaper_start(pid_t pid); + +#endif diff --git a/loader/tests/functional/fixtures/gated_forwarder.sh b/loader/tests/functional/fixtures/gated_forwarder.sh new file mode 100755 index 00000000000..a1fad2e994d --- /dev/null +++ b/loader/tests/functional/fixtures/gated_forwarder.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +set -euo pipefail + +exec /dev/null 2>&1 + +# Bound the wait even if the test runner is killed before releasing the gate. +for ((i = 0; i < 3000; ++i)); do + if [[ -e "${FAKE_FORWARDER_RELEASE_PATH}" ]]; then + echo "${*:2}" >> "${FAKE_FORWARDER_LOG_PATH}" + exit 0 + fi + sleep 0.01 +done +exit 1 diff --git a/loader/tests/functional/fixtures/reaper_shutdown.c b/loader/tests/functional/fixtures/reaper_shutdown.c new file mode 100644 index 00000000000..a294c520ae5 --- /dev/null +++ b/loader/tests/functional/fixtures/reaper_shutdown.c @@ -0,0 +1,115 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static pid_t php_pid; +static pthread_t php_thread; +static char *exit_path; +static atomic_int exiting; +static atomic_int background_dlclose; + +static void check_reapers_at_exit(void); +static int thread_count(void); +static void mark_exit(const char *state); +static void fail(const char *message); + +void *dlopen(const char *filename, int flags) { + // PHP's DEEPBIND would otherwise hide the loader's calls from our dlclose hook. + if (filename) { + const char *name = strrchr(filename, '/'); + if (!strcmp(name ? name + 1 : filename, "dd_library_loader.so")) { + flags &= ~RTLD_DEEPBIND; + } + } + void *(*open_library)(const char *, int) = + (void *(*)(const char *, int))dlsym(RTLD_NEXT, "dlopen"); + if (!open_library) fail("Cannot resolve libc dlopen\n"); + return open_library(filename, flags); +} + +int dlclose(void *handle) { + if (getpid() == php_pid && atomic_load(&exiting) && + !pthread_equal(pthread_self(), php_thread)) { + // Observe the unsafe overlap without letting it corrupt destructor state. + atomic_store(&background_dlclose, 1); + return 0; + } + int (*close_library)(void *) = (int (*)(void *))dlsym(RTLD_NEXT, "dlclose"); + if (!close_library) fail("Cannot resolve libc dlclose\n"); + return close_library(handle); +} + +__attribute__((constructor)) static void register_exit_check(void) { + const char *expected_pid = getenv("DD_REAPER_TEST_PID"); + // The forwarder inherits LD_PRELOAD, but only the PHP process owns this check. + if (!expected_pid || getpid() != (pid_t)strtol(expected_pid, NULL, 10)) return; + php_pid = getpid(); + php_thread = pthread_self(); + // PHP can tear down its environment before libc starts running exit handlers. + const char *path = getenv("DD_REAPER_TEST_EXIT_PATH"); + if (!path || !(exit_path = strdup(path))) fail("Cannot save exit marker path\n"); + if (atexit(check_reapers_at_exit)) fail("Cannot register exit check\n"); +} + +static void check_reapers_at_exit(void) { + if (getpid() != php_pid) return; + if (thread_count() <= 1) fail("No telemetry reapers active at process exit\n"); + + // PHP has shut down its modules. Tell the test it can release the forwarders, + // and keep this real exit handler active until their reaper threads finish. + atomic_store(&exiting, 1); + mark_exit("entered\n"); + struct timespec start, current; + if (clock_gettime(CLOCK_MONOTONIC, &start)) fail("Cannot read clock\n"); + while (thread_count() > 1) { + if (clock_gettime(CLOCK_MONOTONIC, ¤t)) fail("Cannot read clock\n"); + if (current.tv_sec - start.tv_sec >= 5) fail("Telemetry reapers did not finish\n"); + struct timespec delay = {0, 1000000}; + nanosleep(&delay, NULL); + } + if (atomic_load(&background_dlclose)) { + fail("Telemetry reaper called dlclose while an atexit handler was running\n"); + } + // Thread exit alone is insufficient: every telemetry child must be reaped. + siginfo_t child; + if (waitid(P_ALL, 0, &child, WEXITED | WNOHANG | WNOWAIT) != -1 || errno != ECHILD) { + fail("Telemetry children remain after their reapers exited\n"); + } + mark_exit("passed\n"); + free(exit_path); +} + +static int thread_count(void) { + DIR *tasks = opendir("/proc/self/task"); + if (!tasks) fail("Cannot inspect PHP threads\n"); + int count = 0; + struct dirent *task; + while ((task = readdir(tasks))) { + if (task->d_name[0] != '.') ++count; + } + closedir(tasks); + return count; +} + +static void mark_exit(const char *state) { + int fd = open(exit_path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd < 0) fail("Cannot open exit marker\n"); + size_t length = strlen(state); + if (write(fd, state, length) != (ssize_t)length) fail("Cannot write exit marker\n"); + if (close(fd)) fail("Cannot close exit marker\n"); +} + +static void fail(const char *message) { + ssize_t written = write(STDERR_FILENO, message, strlen(message)); + (void)written; + _exit(86); +} diff --git a/loader/tests/functional/test_telemetry_reaper_shutdown.php b/loader/tests/functional/test_telemetry_reaper_shutdown.php new file mode 100644 index 00000000000..633e9b23880 --- /dev/null +++ b/loader/tests/functional/test_telemetry_reaper_shutdown.php @@ -0,0 +1,119 @@ +/dev/null'), 'glibc ') !== 0) { + echo "Skip: test requires glibc exit handlers\n"; + exit(0); +} + +$telemetryLogPath = tempnam(sys_get_temp_dir(), 'test_loader_'); +$outputPath = tempnam(sys_get_temp_dir(), 'test_loader_'); +$errorPath = tempnam(sys_get_temp_dir(), 'test_loader_'); +$fixturePath = tempnam(sys_get_temp_dir(), 'test_loader_'); +$releasePath = $telemetryLogPath.'.release'; +$exitPath = $telemetryLogPath.'.exit'; +$process = null; + +try { + $compiler = getenv('CC') ?: trim((string) shell_exec('command -v cc || command -v clang')); + if ($compiler === '') { + throw new \Exception('A C compiler is required for the shutdown fixture'); + } + $compile = sprintf( + '%s -std=gnu11 -O2 -Wall -Wextra -Werror -fPIC -shared -pthread %s -ldl -o %s 2>&1', + escapeshellarg($compiler), + escapeshellarg(__DIR__.'/fixtures/reaper_shutdown.c'), + escapeshellarg($fixturePath) + ); + exec($compile, $compilerOutput, $compilerStatus); + if ($compilerStatus !== 0) { + throw new \Exception('Cannot build shutdown fixture: '.implode("\n", $compilerOutput)); + } + + // An empty package leaves only telemetry children, all held at the gate. + // Count them before shutdown so we can wait for every forwarder afterwards. + $code = '$children = trim(file_get_contents("/proc/self/task/".getmypid()."/children"));'. + 'echo $children === "" ? 0 : count(explode(" ", $children));'; + $command = sprintf( + 'exec env DD_TRACE_DEBUG=0 DD_LOADER_PACKAGE_PATH=/nonexistent-ddloader-test '. + 'LD_PRELOAD=%s DD_REAPER_TEST_PID=$$ DD_REAPER_TEST_EXIT_PATH=%s '. + 'FAKE_FORWARDER_RELEASE_PATH=%s FAKE_FORWARDER_LOG_PATH=%s '. + 'DD_TELEMETRY_FORWARDER_PATH=%s %s -n -dzend_extension=%s -r %s', + escapeshellarg($fixturePath), + escapeshellarg($exitPath), + escapeshellarg($releasePath), + escapeshellarg($telemetryLogPath), + escapeshellarg(__DIR__.'/fixtures/gated_forwarder.sh'), + escapeshellarg(PHP_BINARY), + escapeshellarg(getLoaderAbsolutePath()), + escapeshellarg($code) + ); + // Files avoid mistaking an inherited output pipe for PHP still running. + $process = proc_open($command, [ + 0 => ['file', '/dev/null', 'r'], + 1 => ['file', $outputPath, 'w'], + 2 => ['file', $errorPath, 'w'], + ], $pipes); + if (!is_resource($process)) { + throw new \Exception('Failed to start PHP process'); + } + + $enteredExit = false; + $deadline = microtime(true) + 10; + do { + clearstatcache(true, $exitPath); + if (!$enteredExit && file_exists($exitPath)) { + // Release telemetry only after Zend shutdown, inside a native exit + // handler. The old reaper then calls dlclose concurrently with it. + $enteredExit = true; + touch($releasePath); + } + $status = proc_get_status($process); + if (!$status['running']) { + break; + } + usleep(10000); + } while (microtime(true) < $deadline); + + if ($status['running']) { + throw new \Exception('Loader shutdown or telemetry reaping did not finish'); + } + if ($status['exitcode'] !== 0) { + throw new \Exception('PHP failed during loader shutdown: '.$status['exitcode']."\n". + file_get_contents($errorPath)); + } + if (!$enteredExit || file_get_contents($exitPath) !== "passed\n") { + throw new \Exception('Native exit handler did not verify telemetry reaping'); + } + $children = (int) file_get_contents($outputPath); + if ($children <= 0) { + throw new \Exception('No telemetry children were started'); + } + + echo "OK: Telemetry reaped during atexit without a background dlclose\n"; +} finally { + // Let the real child processes finish, including on a regression failure. + touch($releasePath); + if (is_resource($process)) { + proc_close($process); + } + $children = (int) file_get_contents($outputPath); + $deadline = microtime(true) + 5; + do { + $completed = count(file($telemetryLogPath)); + if ($completed >= $children) { + break; + } + usleep(10000); + } while (microtime(true) < $deadline); + @unlink($telemetryLogPath); + @unlink($outputPath); + @unlink($errorPath); + @unlink($fixturePath); + @unlink($releasePath); + @unlink($exitPath); + if ($completed < $children) { + throw new \Exception('Telemetry children did not finish after releasing the gate'); + } +}