dynafu: zero-initialize Voxel::n in TSDFVolumeCPU::reset() - #4179
Conversation
TSDFVolumeCPU::reset() zeros Voxel::v and Voxel::weight but leaves Voxel::n uninitialized. IntegrateInvoker::operator() reads this uninitialized n and passes it straight into WarpField::applyWarp() whenever warpfield->getNodeIndex() is null, i.e. on any integrate() call before the warp field has registered its first node. applyWarp() then indexes WarpField::nodes (unchecked) n times, so garbage left over in the freshly-allocated Mat can produce an out-of-bounds/wild pointer access. This reproduced as an intermittent SIGSEGV inside applyWarp on macOS arm64 CI in a downstream project (OpenCvSharp), confirmed via crash report: EXC_BAD_ACCESS KERN_INVALID_ADDRESS at 0x0, faulting thread inside WarpField::applyWarp called from IntegrateInvoker::operator() via parallel_for_. Adds a regression test that exercises the vulnerable code path (a single synthetic depth frame, no OpenGL or external test data required) - note that since this is an uninitialized-memory bug, the test cannot deterministically reproduce the crash on every run, but it does guard the specific "first frame, no warp nodes yet" scenario going forward.
1a83ff4 to
dac5989
Compare
cv::dynafu::WarpField::applyWarp crashes intermittently (EXC_BAD_ACCESS/SIGSEGV, confirmed via macOS arm64 CI crash report) because TSDFVolumeCPU::reset() leaves Voxel::n uninitialized; IntegrateInvoker reads it and passes it straight into applyWarp() before the warp field has registered any node. The same signature (a long hang followed by an access violation) also hit Windows x64 CI on this PR, consistent with the same uninitialized-memory bug being non-deterministic across platforms. Upstream fix submitted: opencv/opencv_contrib#4179. Re-enable once that (or an equivalent) fix lands in the submodule pin. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@savuor Should it be promoted to the main repo somehow? |
fallenmi
left a comment
There was a problem hiding this comment.
Looks good. The missing initialization is on a real first-frame path: before the warp field has a node index, integration does not overwrite voxel.n, but immediately passes it to WarpField::applyWarp() as the unchecked neighbor count.
I made the failure deterministic by first filling and releasing an allocation with the same 92-byte-per-voxel layout, then running a public-API DynaFu update on a synthetic depth frame. With the exact-base reset behavior (v and weight only), the process segfaults (exit 139). Exact head dac59890f8c7e0f0100424ad98c9d042f2c51eb0 completes the identical poisoned-allocation oracle 20/20 times after resetting n to zero.
The submitted first-frame regression passes 10/10 repetitions, and the complete enabled RGB-D test binary passes 8/8 tests. git diff --check is clean, and the change merges cleanly with current 5.x. The only published status is an old default error without a usable current matrix, so this approval is based on the exact local source/build reproduction above.
Reviewed with OpenAI Codex assistance; I independently verified the exact source path, base/head behavior, current merge, and tests.
TSDFVolumeCPU::reset()zerosVoxel::vandVoxel::weightbut leavesVoxel::nuninitialized.IntegrateInvoker::operator()reads this uninitializednand passes it straight intoWarpField::applyWarp()wheneverwarpfield->getNodeIndex()is null - i.e. on anyintegrate()call before the warp field has registered its first node.applyWarp()then indexesWarpField::nodes(unchecked)ntimes, so garbage left over in the freshly-allocatedMatcan produce an out-of-bounds/wild pointer access.This reproduced as an intermittent SIGSEGV inside
applyWarpon macOS arm64 CI in a downstream project (OpenCvSharp), confirmed via crash report:EXC_BAD_ACCESS KERN_INVALID_ADDRESS at 0x0, faulting thread insidecv::dynafu::WarpField::applyWarpcalled fromcv::dynafu::IntegrateInvoker::operator()viaparallel_for_(dispatch_apply).Fix: zero-initialize
Voxel::nalongsidev/weightinreset().Added a regression test in
test_dynafu.cppthat exercises the vulnerable code path (a single synthetic depth frame, no OpenGL or external test data required, unlike the module's existing disabledDynamicFusiontests). Since this is an uninitialized-memory bug, the test can't deterministically reproduce the crash on every run, but it does guard the specific "first frame, no warp nodes yet" scenario going forward.Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.