topic_tools / mux: Use after free

[hannob]

I'm testing ROS with Address Sanitizer, a C/C++ memory safety check (part of gcc and clang, enabled with with the -fsanitize=address flag).

When running the test suite for topic_tools I get a report of a use after free error. I'm unfamiliar with the code, so I haven't analyzed it in detail. Stack trace from address sanitizer is below:

==2451==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200000b990 at pc 0x7fd03e081255 bp 0x7fffaee7fe50 sp 0x7fffaee7fe40
READ of size 1 at 0x61200000b990 thread T0
    #0 0x7fd03e081254 in ros::TopicManager::isShuttingDown() /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/include/ros/topic_manager.h:219
    #1 0x7fd03e081254 in ros::TopicManager::unadvertise(std::string const&, boost::shared_ptr<ros::SubscriberCallbacks> const&) /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/topic_manager.cpp:408
    #2 0x7fd03e060b0a in ros::Publisher::Impl::unadvertise() /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/publisher.cpp:54
    #3 0x7fd03e060bf8 in ros::Publisher::Impl::~Impl() /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/publisher.cpp:41
    #4 0x7fd03e061978 in boost::detail::sp_ms_deleter<ros::Publisher::Impl>::destroy() /usr/include/boost/smart_ptr/make_shared_object.hpp:59
    #5 0x7fd03e061978 in boost::detail::sp_ms_deleter<ros::Publisher::Impl>::operator()(ros::Publisher::Impl*) /usr/include/boost/smart_ptr/make_shared_object.hpp:93
    #6 0x7fd03e061978 in boost::detail::sp_counted_impl_pd<ros::Publisher::Impl*, boost::detail::sp_ms_deleter<ros::Publisher::Impl> >::dispose() /usr/include/boost/smart_ptr/detail/sp_counted_impl.hpp:172
    #7 0x42218c in boost::detail::sp_counted_base::release() /usr/include/boost/smart_ptr/detail/sp_counted_base_std_atomic.hpp:110
    #8 0x7fd03d7e2ff7 in __run_exit_handlers (/lib64/libc.so.6+0x3aff7)
    #9 0x7fd03d7e3139 in __GI_exit (/lib64/libc.so.6+0x3b139)
    #10 0x7fd03d7cb4d1 in __libc_start_main (/lib64/libc.so.6+0x234d1)
    #11 0x4174a9 in _start (/var/tmp/portage/dev-ros/topic_tools-1.14.3/work/topic_tools-1.14.3_build/devel/libexec/topic_tools/mux+0x4174a9)

0x61200000b990 is located 208 bytes inside of 304-byte region [0x61200000b8c0,0x61200000b9f0)
freed by thread T0 here:
    #0 0x7fd03e2683bf in operator delete(void*) (/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.4/libasan.so.1+0x583bf)
    #1 0x7fd03d7e2ff7 in __run_exit_handlers (/lib64/libc.so.6+0x3aff7)
    #2 0x7fd03d7e3139 in __GI_exit (/lib64/libc.so.6+0x3b139)
    #3 0x7fd03d7cb4d1 in __libc_start_main (/lib64/libc.so.6+0x234d1)
    #4 0x4174a9 in _start (/var/tmp/portage/dev-ros/topic_tools-1.14.3/work/topic_tools-1.14.3_build/devel/libexec/topic_tools/mux+0x4174a9)

previously allocated by thread T0 here:
    #0 0x7fd03e267ebf in operator new(unsigned long) (/usr/lib/gcc/x86_64-pc-linux-gnu/4.9.4/libasan.so.1+0x57ebf)
    #1 0x7fd03e08b154 in shared_count<ros::TopicManager*, boost::detail::sp_ms_deleter<ros::TopicManager> > /usr/include/boost/smart_ptr/detail/shared_count.hpp:213
    #2 0x7fd03e08b154 in shared_ptr<ros::TopicManager, boost::detail::sp_inplace_tag<boost::detail::sp_ms_deleter<ros::TopicManager> > > /usr/include/boost/smart_ptr/shared_ptr.hpp:388
    #3 0x7fd03e08b154 in boost::detail::sp_if_not_array<ros::TopicManager>::type boost::make_shared<ros::TopicManager>() /usr/include/boost/smart_ptr/make_shared_object.hpp:250
    #4 0x7fd03e07c1c3 in ros::TopicManager::instance() /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/topic_manager.cpp:58
    #5 0x7fd03e13e0af in ros::start() /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/init.cpp:333
    #6 0x7fd03e0daa37 in ros::NodeHandle::construct(std::string const&, bool) /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/node_handle.cpp:174
    #7 0x7fd03e0daf77 in ros::NodeHandle::NodeHandle(std::string const&, std::map<std::string, std::string, std::less<std::string>, std::allocator<std::pair<std::string const, std::string> > > const&) /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/src/libros/node_handle.cpp:84
    #8 0x415ad7 in main /var/tmp/portage/dev-ros/topic_tools-1.14.3/work/ros_comm-1.14.3/tools/topic_tools/src/mux.cpp:301
    #9 0x7fd03d7cb4ca in __libc_start_main (/lib64/libc.so.6+0x234ca)
    #10 0x4174a9 in _start (/var/tmp/portage/dev-ros/topic_tools-1.14.3/work/topic_tools-1.14.3_build/devel/libexec/topic_tools/mux+0x4174a9)

SUMMARY: AddressSanitizer: heap-use-after-free /var/tmp/portage/dev-ros/roscpp-1.14.3/work/ros_comm-1.14.3/clients/roscpp/include/ros/topic_manager.h:219 ros::TopicManager::isShuttingDown()
Shadow bytes around the buggy address:
  0x0c247fff96e0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fff96f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c247fff9700: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
  0x0c247fff9710: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fff9720: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c247fff9730: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fa fa
  0x0c247fff9740: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c247fff9750: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c247fff9760: 00 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa
  0x0c247fff9770: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c247fff9780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==2451==ABORTING

[dirk-thomas]

Please consider to provide a pull request to address the problem.

[cwecht]

I tried to reproduce this with gcc 7 with ubuntu 18.04 and was not able to reproduce this. Nevertheless I thin the issue is cause by the fact, that the TopicManager gets destroyed during the destruction of the NodeHandle in the main-method but the Publishers in global scope try to access the TopicManager during their destruction. Therefore I moved the Publisher in the main-method here. Now pointers are used to access the publishers in other functions. @hannob could you give it a try and check if the asan-issue is still present?

[cwecht]

1630 might fix this.

[gavanderhoorn]

The work that resulted in this issue was supported by ROSIN - ROS-Industrial Quality-Assured Robot Software Components (European Union’s Horizon 2020 research and innovation programme under grant agreement no. 732287).

More information: rosin-project.eu.