[NO-TICKET] Profiling: Verify iseq before calling rb_iseq_path#6014
Conversation
**What does this PR do?** This PR adds an extra check that the `pathobj` of an `iseq` is non-null before we call `rb_iseq_path` on it. **Motivation:** We saw a null-pointer-dereference crash from a customer inside `rb_iseq_path`. Our current suspicion is that the `pathobj` itself might be NULL, and since it's cheap/easy to check, we're adding this extra safety check as: * Best case it fixes the issue * Worst case the issue still exists, and we learn that it's not the `pathobj` being NULL **Additional Notes:** We've seen the following crash from a customer: (see <https://app.datadoghq.com/logs?query=%40error.source_type%3ACrashtracking%20source%3Aruby%20%40error.stack.frames.function%3Arb_iseq_path&from_ts=1780997944044&to_ts=1783589944044>) > SIGSEGV (SEGV_MAPERR) at address 0x0000000000000000. Address is > below the null-page threshold (0x0000000000001000), suggesting a > direct null dereference on a null pointer. Crash in > /usr/local/lib/libruby.so.3.4.8. with the stack trace showing: ``` 0 rb_iseq_path (/usr/src/ruby/iseq.c:1384) 1 sample_thread (datadog_profiling_native_extension/collectors_stack.c:306) ``` Looking at our code: ```c if (buffer->stack_buffer[i].is_ruby_frame) { VALUE name = rb_iseq_base_label(buffer->stack_buffer[i].as.ruby_frame.iseq); VALUE filename = rb_iseq_path(buffer->stack_buffer[i].as.ruby_frame.iseq); <--- But clearly we're talking about this line name_slice = NIL_P(name) ? DDOG_CHARSLICE_C("") : char_slice_from_ruby_string(name); <-- Line 306 is here ``` Two relevant details here are: we call `rb_iseq_base_label` before `rb_iseq_path` so probably it's not the `iseq` nor its `body` that are NULL: ```c rb_iseq_base_label(const rb_iseq_t *iseq) { return ISEQ_BODY(iseq)->location.base_label; } VALUE rb_iseq_path(const rb_iseq_t *iseq) { return pathobj_path(ISEQ_BODY(iseq)->location.pathobj); } ``` ...and thus we suspect it's either the `pathobj` that's NULL (0x0) or something that the `pathobj` references. It's not clear from looking at the Ruby 3.4.8 sources in what situation an iseq that's live on the stack might ever have a NULL. We've seen around ~100 crashes referencing `rb_iseq_path` from the same customer in the last month; + 1 crash from a different customer that's slightly similar: ``` 0 rb_iseq_path (/usr/src/ruby/iseq.c:1384) 1 control_frame_dump (src/ruby/vm_dump.c:133) 2 rb_vm_bugreport (src/ruby/vm_dump.c:1134) 3 rb_bug (usr/src/ruby/error.c:1115) 4 read_barrier_signal (gc/default/default.c:3442) 5 __kernel_rt_sigreturn 6 7 8 9 10 11 12 grpc_1.0 13 rb_gc_obj_free (/usr/src/ruby/gc.c:1364) 14 gc_compact_plane (gc/default/default.c:5603) 15 gc_sweep (gc/default/default.c:4144) 16 gc_start (gc/default/default.c:6480) 17 rb_gc_prepare_heap (/usr/src/ruby/gc.c:3213) 18 proc_warmup (src/ruby/process.c:8861) ``` Here, Ruby itself is crashing during a crash when trying to access `rb_iseq_path`. One customer was on Ruby 3.4.8 and the other on 3.4.9 so that's interesting but with such a small sample size doesn't say a lot. **How to test the change?** Adding unit test coverage for this one would be... really awkward; this change is small enough that we'll have to validate it as-is.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: d54994d | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-07-09 10:08:32 Comparing candidate commit d54994d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 1 unstable metrics.
|
What does this PR do?
This PR adds an extra check that the
pathobjof aniseqis non-null before we callrb_iseq_pathon it.Motivation:
We saw a null-pointer-dereference crash from a customer inside
rb_iseq_path. Our current suspicion is that thepathobjitself might be NULL, and since it's cheap/easy to check, we're adding this extra safety check as:pathobjbeing NULLChange log entry
Yes. Profiling: Add workaround for crash when collecting frame information
Additional Notes:
We've seen the following crash from a customer:
(see
https://app.datadoghq.com/logs?query=%40error.source_type%3ACrashtracking%20source%3Aruby%20%40error.stack.frames.function%3Arb_iseq_path&from_ts=1780997944044&to_ts=1783589944044)
with the stack trace showing:
Looking at our code:
Two relevant details here are: we call
rb_iseq_base_labelbeforerb_iseq_pathso probably it's not theiseqnor itsbodythat are NULL:...and thus we suspect it's either the
pathobjthat's NULL (0x0) or something that thepathobjreferences.It's not clear from looking at the Ruby 3.4.8 sources in what situation an iseq that's live on the stack might ever have a NULL.
We've seen around ~100 crashes referencing
rb_iseq_pathfrom the same customer in the last month; + 1 crash from a different customer that's slightly similar:Here, Ruby itself is crashing during a crash when trying to access
rb_iseq_path. One customer was on Ruby 3.4.8 and the other on 3.4.9 so that's interesting but with such a small sample size doesn't say a lot.How to test the change?
Adding unit test coverage for this one would be... really awkward; this change is small enough that we'll have to validate it as-is.