Although there is release behavior for the dyn_param_handler_ in the destructor of InflationLayer, but it can't shutdown the callback thread of this handler.
Such conclusion has been proved by the experiment described in another issue #4496
so that the thread of this handler may keep executing after the on_cleanup() of costmap_ros_
however, if here's a callback task being executed by dyn_param_handler_, it would access the released resource in inflation_layer_, and UAF-bug occurs.
suggestion
Similarly, perhaps we need to update the way to completely shutdown dyn_param_handler_ of inflation_layer_?
Bug report
Required Info:
Steps to reproduce issue
Bug happened in my normal usage.
Launch the navigation2 as following steps:
Expected behavior
no bug occured.
Actual behavior
the Asan report of this bug is as following:
Additional information
It's a shutdown-issue
First, based on my execution logs, I can confirm this is a shutdown issue.
It is caused by the
dyn_param_handler_
in theinflation_layer_
not being actively closed.InflationLayer::dynamicParametersCallback()
that triggers the bug is executed by thedynamic_handler_
ofinflation_layer_
:https://github.com/ros-navigation/navigation2/blob/a6b3de13f3c26f9df43ae45065c6a9a48053cccb/nav2_costmap_2d/plugins/inflation_layer.cpp#L105-L109
dyn_param_handler_
in the destructor ofInflationLayer
, but it can't shutdown the callback thread of this handler.Such conclusion has been proved by the experiment described in another issue #4496
on_cleanup()
ofcostmap_ros_
https://github.com/ros-navigation/navigation2/blob/9fbae3e66ecc3b7315ca7cbb070468a564f5e111/nav2_costmap_2d/plugins/inflation_layer.cpp#L79-L83
on_cleanup()
process of thecostmap_ros_
node,inflation_layer_
is released:https://github.com/ros-navigation/navigation2/blob/9fbae3e66ecc3b7315ca7cbb070468a564f5e111/nav2_costmap_2d/src/costmap_2d_ros.cpp#L364
however, if here's a callback task being executed by
dyn_param_handler_
, it would access the released resource ininflation_layer_
, and UAF-bug occurs.suggestion
Similarly, perhaps we need to update the way to completely shutdown
dyn_param_handler_
ofinflation_layer_
?