From 048bc4d5c046689f2fb4697a5b41724dbf539767 Mon Sep 17 00:00:00 2001 From: Pavel Guzenfeld Date: Sun, 7 Jun 2026 00:55:33 +0300 Subject: [PATCH] Bounds-check EventHandlerBase::is_ready() against the wait set (#2376) is_ready() indexed wait_set.events with wait_set_event_index_, which is only assigned once the handler is added to that wait set via add_to_wait_set(). If is_ready() runs first (e.g. a publisher/subscription event thread racing node setup), the index is uninitialized or refers to a different wait set, so the access reads out of bounds and can segfault (observed with ASan during lifecycle configure). Initialize wait_set_event_index_ and make is_ready() return false when the index is not within the wait set's events instead of dereferencing it. Initializing alone is not sufficient (index 0 may still not be this handler's slot), so the bounds check is the actual guard. Adds a regression test that calls is_ready() on a handler that was never added to a wait set. Refs #2376 Co-authored-by: Mike Wake Generated-by: Claude Opus 4.8 (Anthropic) Signed-off-by: Pavel Guzenfeld Signed-off-by: Mike Wake --- rclcpp/include/rclcpp/event_handler.hpp | 5 ++++- rclcpp/src/rclcpp/event_handler.cpp | 7 +++++++ rclcpp/test/rclcpp/test_qos_event.cpp | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/rclcpp/include/rclcpp/event_handler.hpp b/rclcpp/include/rclcpp/event_handler.hpp index cf9ca04121..264046199e 100644 --- a/rclcpp/include/rclcpp/event_handler.hpp +++ b/rclcpp/include/rclcpp/event_handler.hpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -247,7 +248,9 @@ class EventHandlerBase : public Waitable std::function on_new_event_callback_{nullptr}; rcl_event_t event_handle_; - size_t wait_set_event_index_; + // Sentinel that is always out of range, so is_ready() reports "not in any wait set" + // until add_to_wait_set() assigns the real index. See ros2/rclcpp#2376. + size_t wait_set_event_index_ = std::numeric_limits::max(); }; template diff --git a/rclcpp/src/rclcpp/event_handler.cpp b/rclcpp/src/rclcpp/event_handler.cpp index 630bc26d33..f1ef7383ec 100644 --- a/rclcpp/src/rclcpp/event_handler.cpp +++ b/rclcpp/src/rclcpp/event_handler.cpp @@ -69,6 +69,13 @@ EventHandlerBase::add_to_wait_set(rcl_wait_set_t & wait_set) bool EventHandlerBase::is_ready(const rcl_wait_set_t & wait_set) { + // wait_set_event_index_ is only valid once this handler has been added to this wait + // set via add_to_wait_set(). If is_ready() is reached before that (e.g. an event + // thread racing node setup, see ros2/rclcpp#2376), the index may not refer to a slot + // in this wait set, so guard against an out-of-bounds read. + if (wait_set_event_index_ >= wait_set.size_of_events) { + return false; + } return wait_set.events[wait_set_event_index_] == &event_handle_; } diff --git a/rclcpp/test/rclcpp/test_qos_event.cpp b/rclcpp/test/rclcpp/test_qos_event.cpp index 5cfa6b4d60..7a84d526b5 100644 --- a/rclcpp/test/rclcpp/test_qos_event.cpp +++ b/rclcpp/test/rclcpp/test_qos_event.cpp @@ -338,6 +338,28 @@ TEST_F(TestQosEvent, add_to_wait_set) { } } +// Regression for ros2/rclcpp#2376: is_ready() must not index wait_set.events with an +// uninitialized/stale wait_set_event_index_ when the handler is not in the wait set +// (e.g. an event thread racing node setup). It must return false, not read out of bounds. +TEST_F(TestQosEvent, is_ready_when_not_in_wait_set) { + auto publisher = node->create_publisher(topic_name, 10); + auto rcl_handle = publisher->get_publisher_handle(); + + auto callback = [](int) {}; + + const rcl_publisher_event_type_t event_type = + !rclcpp::PublisherBase::event_type_is_supported(RCL_PUBLISHER_OFFERED_DEADLINE_MISSED) ? + RCL_PUBLISHER_MATCHED : RCL_PUBLISHER_OFFERED_DEADLINE_MISSED; + + rclcpp::EventHandler handler( + callback, rcl_publisher_event_init, rcl_handle, event_type); + + // The handler has not been added to any wait set, so its event index is not valid for + // this (empty) wait set; is_ready() must report not-ready instead of dereferencing it. + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + EXPECT_FALSE(handler.is_ready(wait_set)); +} + TEST_F(TestQosEvent, test_on_new_event_callback) { if (!rclcpp::SubscriptionBase::event_type_is_supported(