Intermittent use after free segfault in IntrinsicScaleOffsetDisto.hpp due to mixing shared and raw pointers

[SFN-eu]

Intermittently, when using aliceVision modules that need to read sfm data (in my case depthMapEstimation), readCamera (in src/aliceVision/sfmDataIO/AlembicImporter.cpp) down a long chain of function calls eventually tries to dereference a shared pointer with user count zero and then crashes due to to an invalid read.

In the end, I think this is down to two problems

1. aliceVision::camera::angleBetweenRays, aliceVision::camera::applyIntrinsicExtrinsic and higher up the chain using raw pointers (to make matters worse, in many cases they are shared pointers that have then been casted to raw pointers), where aliceVision::camera::IntrinsicScaleOffsetDisto expects shared pointers.
2. aliceVision::camera::IntrinsicScaleOffsetDisto and other functions in the same file compare if the pointer is null, not if the pointer has use_count zero. This leads to incorrect dereference against non-zero values stored in the pointer. See https://en.cppreference.com/w/cpp/memory/shared_ptr/use_count.html.

Valgrind output, immediately followed by segfault:

==760953== Invalid read of size 8
==760953==    at 0x67D9AAB: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_destroy() (shared_ptr_base.h:143)
==760953==    by 0x9F41A03: aliceVision::camera::IntrinsicScaleOffsetDisto::removeDistortion(Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) const (IntrinsicScaleOffsetDisto.hpp:118)
==760953==    by 0x82E8F55: aliceVision::camera::applyIntrinsicExtrinsic(aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) (IntrinsicBase.hpp:501)
==760953==    by 0x82E916B: aliceVision::camera::angleBetweenRays(aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) (IntrinsicBase.hpp:535)
==760953==    by 0x82E5DD3: aliceVision::mvsUtils::MultiViewParams::findNearestCamsFromLandmarks(int, int) const (MultiViewParams.cpp:554)
==760953==    by 0x7B2458D: aliceVision::depthMap_sycl::DepthMapEstimator::getTilesList(std::vector<int, std::allocator<int> > const&, std::vector<aliceVision::depthMap_sycl::Tile, std::allocator<aliceVision::depthMap_sycl::Tile> >&) (src/aliceVision/depthMap_sycl/DepthMapEstimator.cpp:70)
==760953==    by 0x7B2068D: aliceVision::depthMap_sycl::DepthMapEstimator::compute(hipsycl::sycl::queue&, std::vector<int, std::allocator<int> > const&) (src/aliceVision/depthMap_sycl/DepthMapEstimator.cpp:116)
==760953==    by 0x7B0A449: aliceVision::depthMap_sycl::computeOnMultiDevices(std::vector<int, std::allocator<int> > const&, aliceVision::depthMap_sycl::IDeviceJob&, int) [clone .omp_outlined_debug__] (src/aliceVision/depthMap_sycl/computeOnMultiDevices.cpp:124)
==760953==    by 0x7B0A716: aliceVision::depthMap_sycl::computeOnMultiDevices(std::vector<int, std::allocator<int> > const&, aliceVision::depthMap_sycl::IDeviceJob&, int) [clone .omp_outlined] (src/aliceVision/depthMap_sycl/computeOnMultiDevices.cpp:111)
==760953==    by 0x9A66E38: __kmp_invoke_microtask (in /usr/lib/libomp.so)
==760953==    by 0x99B2CA7: ??? (in /usr/lib/libomp.so)
==760953==    by 0x99B7007: ??? (in /usr/lib/libomp.so)
==760953==  Address 0x199e0c80 is 0 bytes inside a block of size 56 free'd
==760953==    at 0x4A9EA4D: operator delete(void*, unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==760953==    by 0x9F1F22A: std::__new_allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> >::deallocate(std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>*, unsigned long) (new_allocator.h:172)
==760953==    by 0x9F1CD55: deallocate (allocator.h:215)
==760953==    by 0x9F1CD55: deallocate (alloc_traits.h:649)
==760953==    by 0x9F1CD55: std::__allocated_ptr<std::allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> > >::~__allocated_ptr() (allocated_ptr.h:74)
==760953==    by 0x9F216D3: std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionRadialK3, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_destroy() (shared_ptr_base.h:625)
==760953==    by 0x9F41A03: aliceVision::camera::IntrinsicScaleOffsetDisto::removeDistortion(Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) const (IntrinsicScaleOffsetDisto.hpp:118)
==760953==    by 0x82E8F55: aliceVision::camera::applyIntrinsicExtrinsic(aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) (IntrinsicBase.hpp:501)
==760953==    by 0x82E9153: aliceVision::camera::angleBetweenRays(aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, aliceVision::geometry::Pose3 const&, aliceVision::camera::IntrinsicBase const*, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&, Eigen::Matrix<double, 2, 1, 0, 2, 1> const&) (IntrinsicBase.hpp:534)
==760953==    by 0x82E5DD3: aliceVision::mvsUtils::MultiViewParams::findNearestCamsFromLandmarks(int, int) const (MultiViewParams.cpp:554)
==760953==    by 0x7B2458D: aliceVision::depthMap_sycl::DepthMapEstimator::getTilesList(std::vector<int, std::allocator<int> > const&, std::vector<aliceVision::depthMap_sycl::Tile, std::allocator<aliceVision::depthMap_sycl::Tile> >&) (src/aliceVision/depthMap_sycl/DepthMapEstimator.cpp:70)
==760953==    by 0x7B2068D: aliceVision::depthMap_sycl::DepthMapEstimator::compute(hipsycl::sycl::queue&, std::vector<int, std::allocator<int> > const&) (src/aliceVision/depthMap_sycl/DepthMapEstimator.cpp:116)
==760953==    by 0x7B0A449: aliceVision::depthMap_sycl::computeOnMultiDevices(std::vector<int, std::allocator<int> > const&, aliceVision::depthMap_sycl::IDeviceJob&, int) [clone .omp_outlined_debug__] (src/aliceVision/depthMap_sycl/computeOnMultiDevices.cpp:124)
==760953==    by 0x7B0A716: aliceVision::depthMap_sycl::computeOnMultiDevices(std::vector<int, std::allocator<int> > const&, aliceVision::depthMap_sycl::IDeviceJob&, int) [clone .omp_outlined] (src/aliceVision/depthMap_sycl/computeOnMultiDevices.cpp:111)
==760953==  Block was alloc'd at
==760953==    at 0x4A9AFC3: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==760953==    by 0x9F1F1FB: std::__new_allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> >::allocate(unsigned long, void const*) (new_allocator.h:151)
==760953==    by 0x9F1CCA5: allocate (allocator.h:203)
==760953==    by 0x9F1CCA5: allocate (alloc_traits.h:614)
==760953==    by 0x9F1CCA5: std::__allocated_ptr<std::allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> > > std::__allocate_guarded<std::allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> > >(std::allocator<std::_Sp_counted_ptr_inplace<aliceVision::camera::DistortionBrown, std::allocator<void>, (__gnu_cxx::_Lock_policy)2> >&) (allocated_ptr.h:102)
==760953==    by 0x9F1B6A8: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<aliceVision::camera::DistortionRadialK3, std::allocator<void>>(aliceVision::camera::DistortionRadialK3*&, std::_Sp_alloc_shared_tag<std::allocator<void> >) (shared_ptr_base.h:967)
==760953==    by 0x9F1A9AB: std::__shared_ptr<aliceVision::camera::DistortionRadialK3, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>>(std::_Sp_alloc_shared_tag<std::allocator<void> >) (shared_ptr_base.h:1719)
==760953==    by 0x9F19970: std::shared_ptr<aliceVision::camera::DistortionRadialK3>::shared_ptr<std::allocator<void>>(std::_Sp_alloc_shared_tag<std::allocator<void> >) (shared_ptr.h:463)
==760953==    by 0x9F185A7: std::shared_ptr<aliceVision::camera::DistortionRadialK3> std::make_shared<aliceVision::camera::DistortionRadialK3>() (shared_ptr.h:1008)
==760953==    by 0x9F14A5C: aliceVision::camera::createDistortion(aliceVision::camera::EDISTORTION, std::initializer_list<double>) (camera.cpp:38)
==760953==    by 0x9F15044: aliceVision::camera::createIntrinsic(aliceVision::camera::EINTRINSIC, aliceVision::camera::EDISTORTION, aliceVision::camera::EUNDISTORTION, unsigned int, unsigned int, double, double, double, double) (camera.cpp:118)
==760953==    by 0x80231AE: aliceVision::sfmDataIO::readCamera(aliceVision::Version const&, Alembic::Abc::v12::ISchemaObject<Alembic::AbcGeom::v12::ICameraSchema> const&, Imath_3_2::Matrix44<double> const&, aliceVision::sfmData::SfMData&, aliceVision::sfmDataIO::ESfMData, long, bool) (AlembicImporter.cpp:801)
==760953==    by 0x8026C5C: aliceVision::sfmDataIO::visitObject(aliceVision::Version const&, Alembic::Abc::v12::IObject, Imath_3_2::Matrix44<double>, aliceVision::sfmData::SfMData&, aliceVision::sfmDataIO::ESfMData, bool) (AlembicImporter.cpp:1166)
==760953==    by 0x8026CEF: aliceVision::sfmDataIO::visitObject(aliceVision::Version const&, Alembic::Abc::v12::IObject, Imath_3_2::Matrix44<double>, aliceVision::sfmData::SfMData&, aliceVision::sfmDataIO::ESfMData, bool) (AlembicImporter.cpp:1174)

Creating a fix that should a) use shared_ptr's more universally b) correctly check against dereferencing.