fix: four real firmware defects found in adversarial review

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 <noreply@anthropic.com>
This commit is contained in:
Indiana
2026-07-27 15:47:21 +00:00
parent 825f6aa510
commit 31f3f91801
5 changed files with 38 additions and 18 deletions

View File

@@ -897,10 +897,14 @@ esp_err_t rtlsdr_exp_stop(rtlsdr_exp_handle_t h)
h->running = false; h->running = false;
/* Tasks self-delete once they observe h->running == false; give them a /* Tasks self-delete once they observe h->running == false; give them a
* moment. A production version should use task-completion notification * moment. The daemon task blocks up to 1000ms per iteration inside
* instead of a fixed delay — left as a TODO, not hardware-dependent but * usb_host_lib_handle_events() before it re-checks h->running, so this
* still unverified/untuned. */ * delay must exceed that or usb_host_uninstall() below can race a still
vTaskDelay(pdMS_TO_TICKS(500)); * -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 */ usb_host_uninstall(); /* see rtlsdr_exp_start() note re: sole USB client assumption */
return ESP_OK; return ESP_OK;

View File

@@ -8,13 +8,15 @@
// correctly" is where the verification stops. See README.md's "What's // correctly" is where the verification stops. See README.md's "What's
// verified vs. not" section before treating any of this as field-tested. // verified vs. not" section before treating any of this as field-tested.
// //
// Boot sequence: bring up Wi-Fi station mode (device_config.h credentials), // Boot sequence: initialize every registered sensor driver first (so a
// wait (briefly, non-fatally) for an initial connection, initialize every // slow/hanging sensor bus can't block network bring-up -- see
// registered sensor driver, then hand off to the telemetry task, which // sensor_driver.h), then bring up Wi-Fi station mode (device_config.h
// periodically samples the sensor registry and POSTs the results to the // credentials), wait (briefly, non-fatally) for an initial connection, and
// backend. Wi-Fi reconnection and per-cycle "are we online" checks happen // hand off to the telemetry task, which periodically samples the sensor
// independently after this, so a boot-time Wi-Fi hiccup doesn't wedge the // registry and POSTs the results to the backend. Wi-Fi reconnection and
// device -- it just starts reporting once the connection comes up. // 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 "wifi_manager.h"
#include "sensor_registry.h" #include "sensor_registry.h"
@@ -34,6 +36,8 @@ static const char *TAG = "app_main";
void app_main(void) { void app_main(void) {
ESP_LOGI(TAG, "Quantumancy sensor node starting"); ESP_LOGI(TAG, "Quantumancy sensor node starting");
sensor_registry_init_all();
ESP_ERROR_CHECK(wifi_manager_start()); ESP_ERROR_CHECK(wifi_manager_start());
esp_err_t err = wifi_manager_wait_connected(pdMS_TO_TICKS(BOOT_WIFI_WAIT_MS)); 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); "wifi_manager will keep retrying in the background", BOOT_WIFI_WAIT_MS);
} }
sensor_registry_init_all();
telemetry_client_start_task(); telemetry_client_start_task();
ESP_LOGI(TAG, "startup complete, telemetry task running"); ESP_LOGI(TAG, "startup complete, telemetry task running");

View File

@@ -122,6 +122,8 @@ esp_err_t bmp280_init(void) {
err = i2c_master_bus_add_device(s_bus, &dev_cfg, &s_dev); err = i2c_master_bus_add_device(s_bus, &dev_cfg, &s_dev);
if (err != ESP_OK) { if (err != ESP_OK) {
ESP_LOGE(TAG, "i2c_master_bus_add_device failed: %s", esp_err_to_name(err)); 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; return err;
} }
@@ -129,7 +131,7 @@ esp_err_t bmp280_init(void) {
err = read_regs(REG_CHIP_ID, &chip_id, 1); err = read_regs(REG_CHIP_ID, &chip_id, 1);
if (err != ESP_OK) { if (err != ESP_OK) {
ESP_LOGE(TAG, "chip id read failed: %s", esp_err_to_name(err)); ESP_LOGE(TAG, "chip id read failed: %s", esp_err_to_name(err));
return err; goto fail;
} }
if (chip_id != CHIP_ID_EXPECTED) { 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); 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); 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 vTaskDelay(pdMS_TO_TICKS(10)); // datasheet: allow >= 2ms after reset
err = read_calibration(); err = read_calibration();
if (err != ESP_OK) { if (err != ESP_OK) {
ESP_LOGE(TAG, "calibration read failed: %s", esp_err_to_name(err)); ESP_LOGE(TAG, "calibration read failed: %s", esp_err_to_name(err));
return err; goto fail;
} }
s_ready = true; s_ready = true;
ESP_LOGI(TAG, "BMP280 init ok (chip id 0x%02x)", chip_id); ESP_LOGI(TAG, "BMP280 init ok (chip id 0x%02x)", chip_id);
return ESP_OK; 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, // Bosch datasheet 3.11.3 double-precision reference compensation formulas,

View File

@@ -73,6 +73,8 @@ esp_err_t mems_mic_init(void) {
err = i2s_channel_init_std_mode(s_rx_chan, &std_cfg); err = i2s_channel_init_std_mode(s_rx_chan, &std_cfg);
if (err != ESP_OK) { if (err != ESP_OK) {
ESP_LOGE(TAG, "i2s_channel_init_std_mode failed: %s", esp_err_to_name(err)); 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); free(s_sample_buf);
s_sample_buf = NULL; s_sample_buf = NULL;
return err; return err;
@@ -81,6 +83,8 @@ esp_err_t mems_mic_init(void) {
err = i2s_channel_enable(s_rx_chan); err = i2s_channel_enable(s_rx_chan);
if (err != ESP_OK) { if (err != ESP_OK) {
ESP_LOGE(TAG, "i2s_channel_enable failed: %s", esp_err_to_name(err)); 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); free(s_sample_buf);
s_sample_buf = NULL; s_sample_buf = NULL;
return err; return err;

View File

@@ -12,7 +12,7 @@
// "simple report" output frames, which need no configuration to start // "simple report" output frames, which need no configuration to start
// streaming after power-up. // streaming after power-up.
// //
// Simple report frame, as reconstructed (5 bytes total): // Simple report frame, as reconstructed (6 bytes total):
// [0] 0xAA frame header // [0] 0xAA frame header
// [1] gesture code raw value, meaning not confirmed against an // [1] gesture code raw value, meaning not confirmed against an
// official datasheet — reported as-is in // official datasheet — reported as-is in
@@ -36,7 +36,7 @@ static const char *TAG = "rd03e";
#define RD03E_RX_BUF_SIZE 512 #define RD03E_RX_BUF_SIZE 512
#define RD03E_SCRATCH_SIZE 256 #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_HEADER = 0xAA;
static const uint8_t FRAME_FOOTER[2] = { 0x55, 0x55 }; 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) { if (buf[i] != FRAME_HEADER) {
continue; 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 continue; // not a real header byte, or a corrupted frame
} }
latest.gesture = buf[i + 1]; latest.gesture = buf[i + 1];