Skip to content

Commit

Permalink
intraprocess bond::Bond::BondStatusCB use after free
Browse files Browse the repository at this point in the history
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 <macwake@gmail.com>
  • Loading branch information
ewak committed Jan 10, 2025
1 parent 8de883f commit 64b1eb0
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion nav2_util/src/lifecycle_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand Down

0 comments on commit 64b1eb0

Please sign in to comment.