[BugFix] Fix scan-teardown use-after-free on MorselQueueFactory (backport #76259)#76346
[BugFix] Fix scan-teardown use-after-free on MorselQueueFactory (backport #76259)#76346mergify[bot] wants to merge 1 commit into
Conversation
|
Cherry-pick of c613e0f has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
|
@mergify[bot]: Backport conflict, please reslove the conflict and resubmit the pr |
…port #76259) Backport of #76259 to branch-4.1. branch-4.1 predates the main refactor that moved FragmentContext into be/src/exec/runtime/ and introduced the exec-runtime test file, so the fix is applied to the pre-refactor location: - The member-declaration-order fix (declare _morsel_queue_factories before _pipelines so it is destroyed after them) is applied to be/src/exec/pipeline/fragment_context.h, where FragmentContext is still fully defined on 4.1. - The regression test is recreated as a self-contained 4.1-native test (be/test/exec/pipeline/fragment_context_morsel_uaf_test.cpp) and registered in be/test/CMakeLists.txt. Signed-off-by: sevev <qiangzh95@gmail.com> (cherry picked from commit c613e0f)
f793bb1 to
83ccc73
Compare
|
Resolved the cherry-pick conflict manually. The conflict was a Adapted the fix to the pre-refactor 4.1 layout, preserving the original footprint:
No behavior change; the fix is a member reorder. |
Why I'm doing:
CN/BE nodes crashed repeatedly with a use-after-free during query teardown.
The crash stack is consistently:
FragmentContextdestroys its members in reverse declaration order._morsel_queue_factorieswas declared after_pipelines, so it wasdestroyed first. While
_pipelinesis then being torn down, a scanoperator's
ConnectorChunkSource::close()(run from~ScanOperator)calls back into the
MorselQueueFactoryviaScanOperatorFactory::mark_split_source_morsel_finished()— dereferencingan already-freed object.
The callback in
close()was added by #69095 as a safety net sosplit-source completion is still reported when a chunk source exits
through non-EOF paths (cancel / error / limit reached). On those teardown
paths the factory is gone, so the report becomes a UAF. It only reproduces
for lake/connector scans that use split-source morsels and tear the
fragment down without reaching EOF.
What I'm doing:
Declare
_morsel_queue_factoriesbefore_pipelinesso it is destroyedafter the pipelines and their operators, matching the existing ordering
constraint already documented for
_event_scheduler. This is amember-declaration-order fix only; the relative order versus
_runtime_state/_planis unchanged.Add a regression test (
FragmentContextExecRuntimeTest.MorselQueueFactoryOutlivesPipelines)that installs a tracking
MorselQueueFactoryand a trackingPipelineinto a
FragmentContextand asserts the factory is still alive when thepipeline is destroyed, pinning the destruction order so a future member
reshuffle cannot silently reintroduce the crash.
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check:
This is an automatic backport of pull request #76259 done by [Mergify](https://mergify.com).