[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 <nick@koston.org>
Co-authored-by: J. Nick Koston <nick@home-assistant.io>

esphome/components/canbus/__init__.py

@@ -19,6 +19,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:
@@ -86,8 +89,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),
@@ -108,9 +116,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]

esphome/components/canbus/canbus.cpp

@@ -67,16 +67,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<uint8_t> data;
@@ -100,6 +102,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

esphome/components/canbus/canbus.h

@@ -78,6 +78,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);
   /**
@@ -96,8 +97,9 @@ class Canbus : public Component {
   template<typename... Ts> friend class CanbusSendAction;
   std::vector<CanbusTrigger *> 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<void(uint32_t can_id, bool extended_id, bool rtr, const std::vector<uint8_t> &data)>
       callback_manager_{};
 

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: