Fail test on any detection and print out stacktrace

Author: gyuheon0h

bin_tests/preload/preload.c

@@ -4,10 +4,13 @@
 #define _GNU_SOURCE
 #include <dlfcn.h>
 #include <errno.h>
+#include <execinfo.h>
 #include <fcntl.h>
 #include <pthread.h>
+#include <signal.h>
 #include <stddef.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
@@ -18,6 +21,7 @@ static void (*real_free)(void *) = NULL;
 static void *(*real_calloc)(size_t, size_t) = NULL;
 static void *(*real_realloc)(void *, size_t) = NULL;
 static pthread_once_t init_once = PTHREAD_ONCE_INIT;
+static pthread_once_t detection_once = PTHREAD_ONCE_INIT;
 
 // We should load all the real symbols on library load
 static void init_function_ptrs(void) {
@@ -35,64 +39,65 @@ __attribute__((constructor)) static void preload_ctor(void) {
 
 static int log_fd = -1;
 // Flag to indicate we are currently in the collector; We should only
-// log when we are in the collector.
+// detect allocations when we are in the collector.
 static int collector_marked = 0;
+// Flag to track if we've already detected and reported an allocation
+// This guards against reentrancy of the detection logic.
+static int allocation_detected = 0;
 
-
-static void init_logger(void) {
+// Called by the collector process to enable detection in the collector only
+void dd_preload_logger_mark_collector(void) {
+    collector_marked = 1;
     if (log_fd >= 0 || !collector_marked) {
         // Already initialized or not a collector
         return;
     }
-
-    const char *path = "/tmp/preload_logger.log";
-    log_fd = open(path, O_CREAT | O_WRONLY | O_TRUNC, 0644);
-    if (log_fd >= 0) {
-        char buf[256];
-        pid_t pid = getpid();
-        int len = snprintf(buf, sizeof(buf), "[DEBUG] Collector logger initialized pid=%d, fd=%d, path=%s\n",
-                          pid, log_fd, path);
-        write(log_fd, buf, len);
-    }
-}
-
-// Called by the collector process to scope logging to the collector only
-void dd_preload_logger_mark_collector(void) {
-    collector_marked = 1;
-
-    init_logger();
-
-    if (log_fd >= 0) {
-        char buf[256];
-        pid_t pid = getpid();
-        int len = snprintf(buf, sizeof(buf), "[DEBUG] Marked as collector, pid=%d, fd=%d\n", pid, log_fd);
-        write(log_fd, buf, len);
-    }
 }
 
-static void log_line(const char *tag, size_t size, void *ptr) {
-    if (log_fd < 0) {
-        return;
+static void capture_and_report_allocation(const char *func_name) {
+    // Only report once using atomic compare-and-swap
+    if (__sync_bool_compare_and_swap(&allocation_detected, 0, 1)) {
+        const char *path = "/tmp/preload_detector.log";
+        log_fd = open(path, O_CREAT | O_WRONLY | O_TRUNC, 0644);
+        if (log_fd >= 0) {
+            char buf[4096];
+            pid_t pid = getpid();
+            long tid = syscall(SYS_gettid);
+
+            // Log the detection
+            int len = snprintf(buf, sizeof(buf),
+                "[FATAL] Dangerous allocation detected in collector!\n"
+                "  Function: %s\n"
+                "  PID: %d\n"
+                "  TID: %ld\n"
+                "  Stacktrace:\n",
+                func_name, pid, tid);
+            write(log_fd, buf, len);
+
+            // Capture and log stacktrace
+            void *array[100];
+            int size = backtrace(array, 100);
+            char **strings = backtrace_symbols(array, size);
+
+            if (strings != NULL) {
+                for (int i = 0; i < size; i++) {
+                    len = snprintf(buf, sizeof(buf), "    #%d %s\n", i, strings[i]);
+                    write(log_fd, buf, len);
+                }
+                // Note: we can't use free() here since we're in an allocation hook
+                // backtrace_symbols uses malloc internally, so we have a small leak
+                // but this is acceptable since this only happens once and we guard
+                // against it anyways
+            }
+
+            fsync(log_fd);
+            close(log_fd);
+            log_fd = -1;
+        }
     }
 
-    char buf[256];
-    pid_t pid = getpid();
-    long tid = syscall(SYS_gettid);
-    int len = 0;
-
-    if (strcmp(tag, "malloc") == 0) {
-        len = snprintf(buf, sizeof(buf), "pid=%d tid=%ld malloc size=%zu ptr=%p\n", pid, tid, size, ptr);
-    } else if (strcmp(tag, "calloc") == 0) {
-        len = snprintf(buf, sizeof(buf), "pid=%d tid=%ld calloc size=%zu ptr=%p\n", pid, tid, size, ptr);
-    } else if (strcmp(tag, "realloc") == 0) {
-        len = snprintf(buf, sizeof(buf), "pid=%d tid=%ld realloc size=%zu ptr=%p\n", pid, tid, size, ptr);
-    } else if (strcmp(tag, "free") == 0) {
-        len = snprintf(buf, sizeof(buf), "pid=%d tid=%ld free ptr=%p\n", pid, tid, ptr);
-    }
-
-    if (len > 0) {
-        (void)write(log_fd, buf, (size_t)len);
-    }
+    // Don't abort. let the collector continue so it can finish writing the crash report
+    // The test will check for the log file and fail if allocations were detected
 }
 
 void *malloc(size_t size) {
@@ -101,10 +106,11 @@ void *malloc(size_t size) {
         return NULL;
     }
 
-    void *ptr = real_malloc(size);
     if (collector_marked) {
-        log_line("malloc", size, ptr);
+        capture_and_report_allocation("malloc");
     }
+
+    void *ptr = real_malloc(size);
     return ptr;
 }
 
@@ -113,9 +119,7 @@ void free(void *ptr) {
         return;
     }
 
-    if (collector_marked) {
-        log_line("free", 0, ptr);
-    }
+    // free is generally safe; we'll allow free operations without failing
     real_free(ptr);
 }
 
@@ -125,10 +129,11 @@ void *calloc(size_t nmemb, size_t size) {
         return NULL;
     }
 
-    void *ptr = real_calloc(nmemb, size);
     if (collector_marked) {
-        log_line("calloc", nmemb * size, ptr);
+        capture_and_report_allocation("calloc");
     }
+
+    void *ptr = real_calloc(nmemb, size);
     return ptr;
 }
 
@@ -138,9 +143,10 @@ void *realloc(void *ptr, size_t size) {
         return NULL;
     }
 
-    void *new_ptr = real_realloc(ptr, size);
     if (collector_marked) {
-        log_line("realloc", size, new_ptr);
+        capture_and_report_allocation("realloc");
     }
+
+    void *new_ptr = real_realloc(ptr, size);
     return new_ptr;
 }

bin_tests/tests/crashtracker_bin_test.rs

@@ -180,18 +180,48 @@ fn test_crash_tracking_bin_no_runtime_callback() {
     run_crash_test_with_artifacts(&config, &artifacts_map, &artifacts, validator).unwrap();
 }
 
-// Manual/diagnostic preload logger check. This is ignored by default to keep the
-// regular bin test suite fast and hermetic. Run with:
-// cargo test --test crashtracker_bin_test -- --ignored manual_runtime_preload_logger
-// Log output is written to tmp/preload_logger.log.
+// Test that verifies the collector process doesn't perform dangerous allocations.
+// Uses LD_PRELOAD to detect malloc/calloc/realloc calls in the collector and fails
+// if any are detected, capturing a stacktrace for debugging.
 #[test]
-#[ignore]
-fn manual_runtime_preload_logger() {
-    run_standard_crash_test_refactored(
-        BuildProfile::Release,
+#[cfg_attr(miri, ignore)]
+#[cfg(target_os = "linux")] // LD_PRELOAD is Linux-specific
+fn test_collector_no_allocations() {
+    let config = CrashTestConfig::new(
+        BuildProfile::Debug,
         TestMode::RuntimePreloadLogger,
         CrashType::NullDeref,
     );
+    let artifacts = StandardArtifacts::new(config.profile);
+    let artifacts_map = build_artifacts(&artifacts.as_slice()).unwrap();
+
+    let validator: ValidatorFn = Box::new(|payload, _fixtures| {
+        // First validate that the crash report was properly generated
+        PayloadValidator::new(payload).validate_counters()?;
+
+        let detector_log_path = PathBuf::from("/tmp/preload_detector.log");
+
+        if detector_log_path.exists() {
+            let log_content = fs::read_to_string(&detector_log_path)
+                .context("Failed to read preload detector log")?;
+
+            // Clean up the log file first
+            let _ = fs::remove_file(&detector_log_path);
+
+            eprintln!("=== DANGEROUS ALLOCATION DETECTED IN COLLECTOR ===");
+            eprintln!("{}", log_content);
+
+            anyhow::bail!(
+                "Collector performed dangerous allocation! Check the log above for stacktrace."
+            );
+        }
+
+        // If we get here, no dangerous allocations were detected, we shuld pass
+        Ok(())
+    });
+
+    // Run the test - we expect it to complete normally without the collector aborting
+    run_crash_test_with_artifacts(&config, &artifacts_map, &artifacts, validator).unwrap();
 }
 
 #[test]

libdd-crashtracker/src/collector/emitters.rs

@@ -143,6 +143,10 @@ pub(crate) fn emit_crashreport(
     ucontext: *const ucontext_t,
     ppid: i32,
 ) -> Result<(), EmitterError> {
+    let mut test = Vec::new();
+    test.push(config_str);
+    test.push(metadata_string);
+    test.push("lol");
     // The following order is important in order to emit the crash ping:
     // - receiver expects the config
     // - then message if any