From 64b1eb0aeb3d16102836ce15dda43e0d74c17263 Mon Sep 17 00:00:00 2001 From: Mike Wake Date: Fri, 10 Jan 2025 15:15:57 +1100 Subject: [PATCH] intraprocess bond::Bond::BondStatusCB use after free The Bond mechanism includes creation of a subscription using a reference to a member function(bondStatusCB) of the Bond class. This member function operates on member variables. The lifecycle_node was calling bond_.reset() which releases the memory as far as the lifecycle_node is concerned but this is not immediately released from the rclcpp internal mechanisms (especially intraprocess). As a result the bondStatusCB function can called after it has been freed. This use after free shows up reliably with asan when running the test_bond test. This change allows the test_bond to suceed by calling bond_->breakBond() (rather than bond_.reset()) to break the bond rather than expecting it to be done cleanly by the ~Bond() destructor. Is it enough is TBC. Other possibilities might be to get the Bond to inherit from std::enable_shared_from_this(), as Ros2 Nodes do, so that the pointer to the Bond member function bondStatusCB function remains valid until the subscription is released during destruction. Signed-off-by: Mike Wake --- nav2_util/src/lifecycle_node.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nav2_util/src/lifecycle_node.cpp b/nav2_util/src/lifecycle_node.cpp index 49fdf51ec7..57a520e6f3 100644 --- a/nav2_util/src/lifecycle_node.cpp +++ b/nav2_util/src/lifecycle_node.cpp @@ -149,7 +149,7 @@ void LifecycleNode::destroyBond() RCLCPP_INFO(get_logger(), "Destroying bond (%s) to lifecycle manager.", this->get_name()); if (bond_) { - bond_.reset(); + bond_->breakBond(); } } }