From 3be8c5b6b8f7bd769d1f6a81963021668a1661d1 Mon Sep 17 00:00:00 2001 From: fakuivan Date: Sat, 10 Oct 2026 22:07:36 -0300 Subject: [PATCH] [canbus] Fix high frame rates causing crashes due to unbounded iteration (#14481) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: J. Nick Koston Co-authored-by: J. Nick Koston --- esphome/components/canbus/__init__.py | 20 ++++++++++++++++---- esphome/components/canbus/canbus.cpp | 22 +++++++++++++++------- esphome/components/canbus/canbus.h | 6 ++++-- tests/components/canbus/common.yaml | 1 + 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/esphome/components/canbus/__init__.py b/esphome/components/canbus/__init__.py index 51bda0c75df..23dc41d5092 100644 --- a/esphome/components/canbus/__init__.py +++ b/esphome/components/canbus/__init__.py @@ -20,6 +20,9 @@ CONF_REMOTE_TRANSMISSION_REQUEST = "remote_transmission_request" CONF_CANBUS_ID = "canbus_id" CONF_BIT_RATE = "bit_rate" CONF_ON_FRAME = "on_frame" +CONF_MAX_FRAMES_PER_LOOP = "max_frames_per_loop" +DEFAULT_MAX_FRAMES_PER_LOOP = 50 # Keep in sync with max_frames_per_loop_ in canbus.h +DEFAULT_BIT_RATE = "125KBPS" # Keep in sync with bit_rate_ in canbus.h def validate_id(config: ConfigType) -> ConfigType: @@ -87,8 +90,13 @@ CANBUS_SCHEMA = cv.Schema( { cv.GenerateID(): cv.declare_id(CanbusComponent), cv.Required(CONF_CAN_ID): cv.int_range(min=0, max=0x1FFFFFFF), - cv.Optional(CONF_BIT_RATE, default="125KBPS"): cv.enum(CAN_SPEEDS, upper=True), + cv.Optional(CONF_BIT_RATE, default=DEFAULT_BIT_RATE): cv.enum( + CAN_SPEEDS, upper=True + ), cv.Optional(CONF_USE_EXTENDED_ID, default=False): cv.boolean, + cv.Optional( + CONF_MAX_FRAMES_PER_LOOP, default=DEFAULT_MAX_FRAMES_PER_LOOP + ): cv.positive_not_null_int, cv.Optional(CONF_ON_FRAME): automation.validate_automation( { cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(CanbusTrigger), @@ -109,9 +117,13 @@ CANBUS_SCHEMA.add_extra(validate_id) async def setup_canbus_core_(var: MockObj, config: ConfigType) -> None: await cg.register_component(var, config) - cg.add(var.set_can_id([config[CONF_CAN_ID]])) - cg.add(var.set_use_extended_id([config[CONF_USE_EXTENDED_ID]])) - cg.add(var.set_bitrate(CAN_SPEEDS[config[CONF_BIT_RATE]])) + cg.add(var.set_can_id(config[CONF_CAN_ID])) + if config[CONF_USE_EXTENDED_ID]: + cg.add(var.set_use_extended_id(True)) + if (bit_rate := config[CONF_BIT_RATE]) != DEFAULT_BIT_RATE: + cg.add(var.set_bitrate(CAN_SPEEDS[bit_rate])) + if (max_frames := config[CONF_MAX_FRAMES_PER_LOOP]) != DEFAULT_MAX_FRAMES_PER_LOOP: + cg.add(var.set_max_frames_per_loop(max_frames)) for conf in config.get(CONF_ON_FRAME, []): can_id = conf[CONF_CAN_ID] diff --git a/esphome/components/canbus/canbus.cpp b/esphome/components/canbus/canbus.cpp index 828eb77d1ca..88eed2246fd 100644 --- a/esphome/components/canbus/canbus.cpp +++ b/esphome/components/canbus/canbus.cpp @@ -66,16 +66,18 @@ void Canbus::add_trigger(CanbusTrigger *trigger) { void Canbus::loop() { struct CanFrame can_message; - // read all messages until queue is empty - int message_counter = 0; - while (this->read_message(&can_message) == canbus::ERROR_OK) { - message_counter++; + uint32_t message_counter = 0; + for (; message_counter < this->max_frames_per_loop_; message_counter++) { + if (this->read_message(&can_message) != canbus::ERROR_OK) { + break; + } + if (can_message.use_extended_id) { - ESP_LOGD(TAG, "received can message (#%d) extended can_id=0x%" PRIx32 " size=%d", message_counter, + ESP_LOGD(TAG, "received can message (#%" PRIu32 ") extended can_id=0x%" PRIx32 " size=%d", message_counter + 1, can_message.can_id, can_message.can_data_length_code); } else { - ESP_LOGD(TAG, "received can message (#%d) std can_id=0x%" PRIx32 " size=%d", message_counter, can_message.can_id, - can_message.can_data_length_code); + ESP_LOGD(TAG, "received can message (#%" PRIu32 ") std can_id=0x%" PRIx32 " size=%d", message_counter + 1, + can_message.can_id, can_message.can_data_length_code); } std::vector data; @@ -99,6 +101,12 @@ void Canbus::loop() { } } } + + if (message_counter == this->max_frames_per_loop_) { + ESP_LOGW(TAG, + "Reached max_frames_per_loop=%" PRIu32 ", deferring remaining CAN frames. Some frames might be dropped", + this->max_frames_per_loop_); + } } } // namespace esphome::canbus diff --git a/esphome/components/canbus/canbus.h b/esphome/components/canbus/canbus.h index 8685c026b0b..432eb77e13d 100644 --- a/esphome/components/canbus/canbus.h +++ b/esphome/components/canbus/canbus.h @@ -82,6 +82,7 @@ class Canbus : public Component { void set_can_id(uint32_t can_id) { this->can_id_ = can_id; } void set_use_extended_id(bool use_extended_id) { this->use_extended_id_ = use_extended_id; } void set_bitrate(CanSpeed bit_rate) { this->bit_rate_ = bit_rate; } + void set_max_frames_per_loop(uint32_t max_frames_per_loop) { this->max_frames_per_loop_ = max_frames_per_loop; } void add_trigger(CanbusTrigger *trigger); /** @@ -100,8 +101,9 @@ class Canbus : public Component { template friend class CanbusSendAction; std::vector triggers_{}; uint32_t can_id_; - bool use_extended_id_; - CanSpeed bit_rate_; + uint32_t max_frames_per_loop_{50}; // Keep in sync with DEFAULT_MAX_FRAMES_PER_LOOP in __init__.py + CanSpeed bit_rate_{CAN_125KBPS}; // Keep in sync with DEFAULT_BIT_RATE in __init__.py + bool use_extended_id_{false}; CallbackManager &data)> callback_manager_{}; diff --git a/tests/components/canbus/common.yaml b/tests/components/canbus/common.yaml index 57ee25bfc16..14e0d6bf5a6 100644 --- a/tests/components/canbus/common.yaml +++ b/tests/components/canbus/common.yaml @@ -5,6 +5,7 @@ canbus: tx_pin: 5 can_id: 4 bit_rate: 50kbps + max_frames_per_loop: 100 on_frame: - can_id: 500 then: