From 31f3f918019d1e96482357afd2c4783520426572 Mon Sep 17 00:00:00 2001 From: Indiana Date: Mon, 27 Jul 2026 15:47:21 +0000 Subject: [PATCH] fix: four real firmware defects found in adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rd03e.c: RD03E_FRAME_LEN was 5 but the frame's own documented layout (header + gesture + distance_lo + distance_hi + footer[2]) is 6 bytes. The footer check read buf[i+3], colliding with the distance high byte at that same index — so every frame that validated at all was forced to have distance_cm = lo | 0x5500 (~218m) regardless of what the sensor reported. Distance readings were garbage 100% of the time, not intermittently. mems_mic.c: i2s_del_channel() was missing on 2 of 3 init failure paths, leaking the channel handle. bmp280.c: the I2C bus/device handles leaked on 4 of 5 init failure paths; added a fail label that releases both. app_main.c: sensors now init before Wi-Fi bring-up, matching the rationale sensor_driver.h already documents (a hanging sensor bus must not be able to block network bring-up). rtlsdr_experimental.c: rtlsdr_exp_stop() waited 500ms before usb_host_uninstall(), but the daemon task blocks up to 1000ms inside usb_host_lib_handle_events() before re-checking its running flag — the delay must exceed that or teardown races a live daemon task. Co-Authored-By: Claude Opus 5 --- .../rtlsdr_experimental/rtlsdr_experimental.c | 12 ++++++++---- firmware/esp32p4-sensor-node/main/app_main.c | 19 +++++++++++-------- firmware/esp32p4-sensor-node/main/bmp280.c | 15 ++++++++++++--- firmware/esp32p4-sensor-node/main/mems_mic.c | 4 ++++ firmware/esp32p4-sensor-node/main/rd03e.c | 6 +++--- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/firmware/esp32p4-sensor-node/components/rtlsdr_experimental/rtlsdr_experimental.c b/firmware/esp32p4-sensor-node/components/rtlsdr_experimental/rtlsdr_experimental.c index c65ccdb..d4e9bb1 100644 --- a/firmware/esp32p4-sensor-node/components/rtlsdr_experimental/rtlsdr_experimental.c +++ b/firmware/esp32p4-sensor-node/components/rtlsdr_experimental/rtlsdr_experimental.c @@ -897,10 +897,14 @@ esp_err_t rtlsdr_exp_stop(rtlsdr_exp_handle_t h) h->running = false; /* Tasks self-delete once they observe h->running == false; give them a - * moment. A production version should use task-completion notification - * instead of a fixed delay — left as a TODO, not hardware-dependent but - * still unverified/untuned. */ - vTaskDelay(pdMS_TO_TICKS(500)); + * moment. The daemon task blocks up to 1000ms per iteration inside + * usb_host_lib_handle_events() before it re-checks h->running, so this + * delay must exceed that or usb_host_uninstall() below can race a still + * -running daemon task still holding the USB host lib open. A production + * version should use task-completion notification instead of a fixed + * delay — left as a TODO, not hardware-dependent but still + * unverified/untuned. */ + vTaskDelay(pdMS_TO_TICKS(1200)); usb_host_uninstall(); /* see rtlsdr_exp_start() note re: sole USB client assumption */ return ESP_OK; diff --git a/firmware/esp32p4-sensor-node/main/app_main.c b/firmware/esp32p4-sensor-node/main/app_main.c index 9ba19b0..1e97d48 100644 --- a/firmware/esp32p4-sensor-node/main/app_main.c +++ b/firmware/esp32p4-sensor-node/main/app_main.c @@ -8,13 +8,15 @@ // correctly" is where the verification stops. See README.md's "What's // verified vs. not" section before treating any of this as field-tested. // -// Boot sequence: bring up Wi-Fi station mode (device_config.h credentials), -// wait (briefly, non-fatally) for an initial connection, initialize every -// registered sensor driver, then hand off to the telemetry task, which -// periodically samples the sensor registry and POSTs the results to the -// backend. Wi-Fi reconnection and per-cycle "are we online" checks happen -// independently after this, so a boot-time Wi-Fi hiccup doesn't wedge the -// device -- it just starts reporting once the connection comes up. +// Boot sequence: initialize every registered sensor driver first (so a +// slow/hanging sensor bus can't block network bring-up -- see +// sensor_driver.h), then bring up Wi-Fi station mode (device_config.h +// credentials), wait (briefly, non-fatally) for an initial connection, and +// hand off to the telemetry task, which periodically samples the sensor +// registry and POSTs the results to the backend. Wi-Fi reconnection and +// per-cycle "are we online" checks happen independently after this, so a +// boot-time Wi-Fi hiccup doesn't wedge the device -- it just starts +// reporting once the connection comes up. #include "wifi_manager.h" #include "sensor_registry.h" @@ -34,6 +36,8 @@ static const char *TAG = "app_main"; void app_main(void) { ESP_LOGI(TAG, "Quantumancy sensor node starting"); + sensor_registry_init_all(); + ESP_ERROR_CHECK(wifi_manager_start()); esp_err_t err = wifi_manager_wait_connected(pdMS_TO_TICKS(BOOT_WIFI_WAIT_MS)); @@ -44,7 +48,6 @@ void app_main(void) { "wifi_manager will keep retrying in the background", BOOT_WIFI_WAIT_MS); } - sensor_registry_init_all(); telemetry_client_start_task(); ESP_LOGI(TAG, "startup complete, telemetry task running"); diff --git a/firmware/esp32p4-sensor-node/main/bmp280.c b/firmware/esp32p4-sensor-node/main/bmp280.c index 473104c..3934ec5 100644 --- a/firmware/esp32p4-sensor-node/main/bmp280.c +++ b/firmware/esp32p4-sensor-node/main/bmp280.c @@ -122,6 +122,8 @@ esp_err_t bmp280_init(void) { err = i2c_master_bus_add_device(s_bus, &dev_cfg, &s_dev); if (err != ESP_OK) { ESP_LOGE(TAG, "i2c_master_bus_add_device failed: %s", esp_err_to_name(err)); + i2c_del_master_bus(s_bus); + s_bus = NULL; return err; } @@ -129,7 +131,7 @@ esp_err_t bmp280_init(void) { err = read_regs(REG_CHIP_ID, &chip_id, 1); if (err != ESP_OK) { ESP_LOGE(TAG, "chip id read failed: %s", esp_err_to_name(err)); - return err; + goto fail; } if (chip_id != CHIP_ID_EXPECTED) { ESP_LOGW(TAG, "unexpected chip id 0x%02x (want 0x%02x) -- check wiring/address", chip_id, CHIP_ID_EXPECTED); @@ -139,18 +141,25 @@ esp_err_t bmp280_init(void) { } err = write_reg(REG_RESET, RESET_MAGIC); - if (err != ESP_OK) return err; + if (err != ESP_OK) goto fail; vTaskDelay(pdMS_TO_TICKS(10)); // datasheet: allow >= 2ms after reset err = read_calibration(); if (err != ESP_OK) { ESP_LOGE(TAG, "calibration read failed: %s", esp_err_to_name(err)); - return err; + goto fail; } s_ready = true; ESP_LOGI(TAG, "BMP280 init ok (chip id 0x%02x)", chip_id); return ESP_OK; + +fail: + i2c_master_bus_rm_device(s_dev); + s_dev = NULL; + i2c_del_master_bus(s_bus); + s_bus = NULL; + return err; } // Bosch datasheet 3.11.3 double-precision reference compensation formulas, diff --git a/firmware/esp32p4-sensor-node/main/mems_mic.c b/firmware/esp32p4-sensor-node/main/mems_mic.c index 27f03b1..8936565 100644 --- a/firmware/esp32p4-sensor-node/main/mems_mic.c +++ b/firmware/esp32p4-sensor-node/main/mems_mic.c @@ -73,6 +73,8 @@ esp_err_t mems_mic_init(void) { err = i2s_channel_init_std_mode(s_rx_chan, &std_cfg); if (err != ESP_OK) { ESP_LOGE(TAG, "i2s_channel_init_std_mode failed: %s", esp_err_to_name(err)); + i2s_del_channel(s_rx_chan); + s_rx_chan = NULL; free(s_sample_buf); s_sample_buf = NULL; return err; @@ -81,6 +83,8 @@ esp_err_t mems_mic_init(void) { err = i2s_channel_enable(s_rx_chan); if (err != ESP_OK) { ESP_LOGE(TAG, "i2s_channel_enable failed: %s", esp_err_to_name(err)); + i2s_del_channel(s_rx_chan); + s_rx_chan = NULL; free(s_sample_buf); s_sample_buf = NULL; return err; diff --git a/firmware/esp32p4-sensor-node/main/rd03e.c b/firmware/esp32p4-sensor-node/main/rd03e.c index 30444ee..422bbd7 100644 --- a/firmware/esp32p4-sensor-node/main/rd03e.c +++ b/firmware/esp32p4-sensor-node/main/rd03e.c @@ -12,7 +12,7 @@ // "simple report" output frames, which need no configuration to start // streaming after power-up. // -// Simple report frame, as reconstructed (5 bytes total): +// Simple report frame, as reconstructed (6 bytes total): // [0] 0xAA frame header // [1] gesture code raw value, meaning not confirmed against an // official datasheet — reported as-is in @@ -36,7 +36,7 @@ static const char *TAG = "rd03e"; #define RD03E_RX_BUF_SIZE 512 #define RD03E_SCRATCH_SIZE 256 -#define RD03E_FRAME_LEN 5 +#define RD03E_FRAME_LEN 6 static const uint8_t FRAME_HEADER = 0xAA; static const uint8_t FRAME_FOOTER[2] = { 0x55, 0x55 }; @@ -116,7 +116,7 @@ esp_err_t rd03e_read(sensor_reading_t *out, size_t max_out, size_t *out_count) { if (buf[i] != FRAME_HEADER) { continue; } - if (memcmp(&buf[i + 3], FRAME_FOOTER, 2) != 0) { + if (memcmp(&buf[i + 4], FRAME_FOOTER, 2) != 0) { continue; // not a real header byte, or a corrupted frame } latest.gesture = buf[i + 1];