fix(live-debugger): retire removed probe configs to prevent use-after-free

[Leiyks]

Problem

test_extension_ci: [7.1] (the valgrind master job) fails with a LEAKED TEST SUMMARY on tests/ext/live-debugger/debugger_enable_dynamic_config.phpt. The .mem artifact shows it is not a leak but a use-after-free:

Invalid read of size 1
  ... probe.id.to_utf8_lossy()                    send_data.rs:523
  ddog_send_debugger_diagnostics                  remote_config.rs:786
  dd_probe_mark_active                            tracer/live_debugger.c:226
  dd_span_probe_begin                             tracer/live_debugger.c:239
 Address ... is 0 bytes inside a block of size 1 free'd
  drop String -> Probe -> ... -> Box<(RemoteConfigParsed, MaybeShmLimiter)>
  ddog_process_remote_configs                     components-rs/remote_config.rs:445  (LiveDebugger Remove)

The freed block is size 1 — the 1-byte probe id "1" the test generates.

Root cause

The C side (tracer/live_debugger.c:162, def->probe = *probe) keeps a shallow copy of the FFI ddog_Probe, whose ddog_CharSlice fields (id, capture config, shm limiter, …) borrow strings owned by the boxed RemoteConfigParsed stored in live_debugger.active. The Box<> exists specifically to give that data a stable heap address for these borrows (per its own comment).

On a LiveDebugger Remove (and on an Entry::Occupied re-add) the box was dropped immediately, freeing those strings. But zai_hook_remove can defer the actual hook teardown, so a still-installed probe can fire afterwards and read the freed strings via ddog_send_debugger_diagnostics(&def->probe, …) → UAF.

Latent since the feature landed (f87446c4d, 2024-10); only the valgrind job observes it because plain runs read the freed-but-intact bytes and pass. It is timing-gated, which is why it surfaces in the full shuffled valgrind suite rather than in isolation.

Fix

Retire removed/replaced boxes into a new live_debugger.retired vec instead of dropping them, and free them in ddog_rshutdown_remote_config — by which point all probe hooks are torn down and no PHP user code can fire a probe. retired is cleared every request, so it stays bounded.

Verification

• Builds on PHP 7.1 with the master-pinned libdatadog (6a6d4a535).
• The previously-leaking test passes 10/10 under valgrind with the fix; non-regressing.
• Note: the UAF needs full-suite (shared request-replayer + shuffle) timing and does not reproduce in isolation, so the authoritative signal is the test_extension_ci: [7.1] valgrind job on this branch.

[datadog-datadog-prod-us1]

 

✨ Fix all issues with BitsAI

⚠️ Warnings

🚦 4 Pipeline jobs failed

DataDog/apm-reliability/dd-trace-php | ASAN test_c with multiple observers: [8.3]     

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.0]     

DataDog/apm-reliability/dd-trace-php | test_extension_ci: [7.1]     

View all 4 failed jobs.

ℹ️ Info

No other issues found (see more)

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 54.08% (-0.03%)

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: a7cfd7d | Docs | Datadog PR Page | Give us feedback!}

[pr-commenter]

Benchmarks [ tracer ]

Benchmark execution time: 2026-06-30 15:27:25

Comparing candidate commit a7cfd7d in PR branch leiyks/fix-ci-live-debugger-uaf with baseline commit bffd346 in branch master.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 193 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

• 🟩 = significantly better candidate vs. baseline
• 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:ContextPropagationBench/benchInject64Bit-opcache

• 🟥 execution_time [+503.212ns; +750.788ns] or [+3.616%; +5.396%]

[bwoebi]

My problem with this PR is that it misidentifies the root cause:
There's no reason why the begin hook should be called in the first place on an already removed hook. This should not be possible - zai_hook_continue carefully uses the ht iterators API to that extent and checks for negative hook id, which signals removal.

And end hooks are anyway guarded with def->removed where needed.

I don't see where ddog_process_remote_configs was called from the shortened trace, so that gives me no hint.