diff --git a/tests/ext/span_stack/span_stack_clone_dropped_root.phpt b/tests/ext/span_stack/span_stack_clone_dropped_root.phpt new file mode 100644 index 0000000000..abdea036db --- /dev/null +++ b/tests/ext/span_stack/span_stack_clone_dropped_root.phpt @@ -0,0 +1,38 @@ +--TEST-- +Cloning a span stack must not leave a dangling root_span alias when the root span is dropped (#3943) +--ENV-- +DD_TRACE_AUTO_FLUSH_ENABLED=0 +DD_TRACE_GENERATE_ROOT_SPAN=1 +DD_CODE_ORIGIN_FOR_SPANS_ENABLED=0 +--FILE-- +baggage["k"] = "v"; + +// Open a sub-trace, capture its stack, then close that sub-trace's own root span. The sub-stack now +// has no active span and no root span of its own. +\DDTrace\start_trace_span(); +$sub = \DDTrace\active_stack(); +\DDTrace\close_span(); + +// Cloning must not copy the parent stack's root_span verbatim as a non-owning alias: the clone holds +// no reference to that root span, so the pointer would dangle once it is dropped below. +$clone = clone $sub; + +// Drop the (auto) root span via the runtime config change. +ini_set("datadog.trace.generate_root_span", "0"); + +// Before the fix, starting a new trace whose parent stack is the clone dereferenced the freed root +// span in ddtrace_set_root_span_properties -> ddtrace_inherit_span_properties (heap-use-after-free). +\DDTrace\switch_stack($clone); +\DDTrace\start_trace_span()->name = "after clone"; + +echo "completed without use-after-free\n"; +var_dump(\DDTrace\active_span()->name); + +?> +--EXPECT-- +completed without use-after-free +string(11) "after clone" diff --git a/tracer/functions.c b/tracer/functions.c index c5d0fdb203..23ec3e5291 100644 --- a/tracer/functions.c +++ b/tracer/functions.c @@ -394,7 +394,6 @@ static zend_object *ddtrace_span_stack_clone_obj(zend_object *old_obj) { ddtrace_span_stack *oldstack = (ddtrace_span_stack *)old_obj; if (oldstack->parent_stack) { // if this is false, we're copying an initial stack stack->root_stack = stack->parent_stack->root_stack; - stack->root_span = stack->parent_stack->root_span; } if (oldstack->root_stack == oldstack) { stack->root_stack = stack; @@ -407,10 +406,9 @@ static zend_object *ddtrace_span_stack_clone_obj(zend_object *old_obj) { } if (pspan) { ZVAL_OBJ_COPY(&stack->property_active, &pspan->std); + stack->root_span = SPANDATA(pspan)->root; } else { - if (oldstack->root_span && oldstack->root_span->stack == oldstack) { - stack->root_span = NULL; - } + stack->root_span = NULL; stack->active = NULL; ZVAL_NULL(&stack->property_active); } @@ -2398,6 +2396,11 @@ PHP_FUNCTION(DDTrace_try_drop_span) { RETURN_FALSE; } + // Assert span stack for manual instantiations of SpanStack/clones. + if (!span->stack || span->stack->active != &span->props) { + RETURN_FALSE; + } + bool on_active_stack = active_stack == span->stack; if (!on_active_stack) { GC_ADDREF(&active_stack->std);