Harden cleanup and reduce internal RAM use. Phase 8C nearly validated

and somewhat stable.
This commit is contained in:
2026-08-31 03:33:43 +02:00
parent 868e9ebc23
commit 6ad6c00d68
12 changed files with 169 additions and 68 deletions
+1 -1
View File
@@ -190,5 +190,5 @@ Hardware diagnostics are synchronous console commands. RS-232 tests own the phys
- Library/hardware ownership is centralized: serial task owns UART1 while running, display service owns I2C/framebuffer, the SSH owner task owns post-initialization wolfSSH runtime calls, and the console dispatcher owns `esp_console_run()`. - Library/hardware ownership is centralized: serial task owns UART1 while running, display service owns I2C/framebuffer, the SSH owner task owns post-initialization wolfSSH runtime calls, and the console dispatcher owns `esp_console_run()`.
- Password authentication performs PBKDF2 outside the user-database mutex and revalidates afterward. Some password mutation paths currently derive verifiers while holding the mutation lock; do not generalize the authentication locking pattern without checking the exact path. - Password authentication performs PBKDF2 outside the user-database mutex and revalidates afterward. Some password mutation paths currently derive verifiers while holding the mutation lock; do not generalize the authentication locking pattern without checking the exact path.
- Avoid holding service/database/broker locks across I2C, network sends, or other potentially long operations unless the existing contract explicitly requires it. Preserve the existing broker-before-serial lock order. - Avoid holding service/database/broker locks across I2C, network sends, or other potentially long operations unless the existing contract explicitly requires it. Preserve the existing broker-before-serial lock order.
- Broker and selected cryptographic allocations prefer PSRAM but can fall back to internal RAM. FreeRTOS control structures and task stacks intentionally remain internal where flash/cache-disable safety matters. - Serial RX/TX stream payloads, broker per-client payloads, the transactional user-database candidate, and selected cryptographic allocations prefer PSRAM with internal fallback. The live user database, FreeRTOS control structures, UART driver buffers, and task stacks remain internal where deterministic/cache-disable access matters.
- The build disables wolfSSL ESP32 AES/SHA acceleration, and the HTTPS path uses software AES for PSRAM-backed records. This preserves the validated workaround for uncoordinated mbedTLS/wolfSSL hardware-crypto locks and a prior mbedTLS external-RAM DMA watchdog stall. - The build disables wolfSSL ESP32 AES/SHA acceleration, and the HTTPS path uses software AES for PSRAM-backed records. This preserves the validated workaround for uncoordinated mbedTLS/wolfSSL hardware-crypto locks and a prior mbedTLS external-RAM DMA watchdog stall.
+2 -2
View File
@@ -30,7 +30,7 @@ This is a semantic map, not a complete file inventory. Start here, then read the
- Normal data caller: `session_broker`; USB, WebSocket, role-`user` SSH, console, and local UI also call serial lifecycle/configuration APIs as appropriate - Normal data caller: `session_broker`; USB, WebSocket, role-`user` SSH, console, and local UI also call serial lifecycle/configuration APIs as appropriate
- Dependencies: ESP-IDF UART driver, `board_pins.h`, NVS - Dependencies: ESP-IDF UART driver, `board_pins.h`, NVS
- Ownership: the cooperative owner arbitrates active diagnostics (`PHASE0`) against the service (`SERVICE`); boot-time static-safe GPIO initialization and service-owned static-mode restoration are explicit exceptions. Unsafe cleanup marks `FAULT` until reboot. - Ownership: the cooperative owner arbitrates active diagnostics (`PHASE0`) against the service (`SERVICE`); boot-time static-safe GPIO initialization and service-owned static-mode restoration are explicit exceptions. Unsafe cleanup marks `FAULT` until reboot.
- Lifecycle: stop/reconfiguration discards serial-service RX/TX and task-local pending bytes, but leaves broker clients, writer ownership, events, and already-fanned output intact. - Lifecycle: stop/reconfiguration discards serial-service RX/TX and task-local pending bytes, but leaves broker clients, writer ownership, events, and already-fanned output intact. The 16 KiB RX and 8 KiB TX payloads prefer PSRAM; their FreeRTOS controls and UART driver storage remain internal.
## Session broker ## Session broker
@@ -91,7 +91,7 @@ This is a semantic map, not a complete file inventory. Start here, then read the
- Interfaces: init/migration/recovery, authenticate, principal-currentness, account/password/role/key mutations, snapshots - Interfaces: init/migration/recovery, authenticate, principal-currentness, account/password/role/key mutations, snapshots
- Called by: web and SSH authentication/currentness checks and console administration - Called by: web and SSH authentication/currentness checks and console administration
- Dependencies: NVS, secure random, mbedTLS cryptography; after a committed command-layer mutation, best-effort web/SSH revocation calls supplement authoritative transport currentness checks - Dependencies: NVS, secure random, mbedTLS cryptography; after a committed command-layer mutation, best-effort web/SSH revocation calls supplement authoritative transport currentness checks
- Ownership: database mutex protects live records; password authentication runs PBKDF2 outside it and revalidates afterward, while mutation locking must be checked per operation. - Ownership: database mutex protects the internal live record and PSRAM-preferred transactional candidate; password authentication runs PBKDF2 outside the mutex and revalidates afterward, while mutation locking must be checked per operation.
- Authorization: UART0 exclusively owns initial administrator bootstrap and unavailable-database recovery; current admins may use admin SSH for other commands unless handler policy denies them. HTTPS currently treats both roles alike. - Authorization: UART0 exclusively owns initial administrator bootstrap and unavailable-database recovery; current admins may use admin SSH for other commands unless handler policy denies them. HTTPS currently treats both roles alike.
- Constraint: final administrator cannot be deleted or demoted; transport principals must be rechecked after mutations. - Constraint: final administrator cannot be deleted or demoted; transport principals must be rechecked after mutations.
+9 -2
View File
@@ -13,6 +13,15 @@ Based on checked-in source plus `README.md` and `docs/roadmap.md`:
- Security/production hardening, OTA, BLE evaluation, advanced networking, and optional filesystem features remain future roadmap work. - Security/production hardening, OTA, BLE evaluation, advanced networking, and optional filesystem features remain future roadmap work.
- Reserved OTA, coredump, NVS-key, and storage partitions do not imply those runtime features are implemented. - Reserved OTA, coredump, NVS-key, and storage partitions do not imply those runtime features are implemented.
## Recent memory audit
- Fixed failed-initialization ownership leaks for wolfSSH, partial HTTPS startup, and TinyUSB teardown. Failed teardown now retains ownership and blocks unsafe duplicate initialization.
- Serial-service RX/TX stream payloads (16 KiB and 8 KiB effective capacity) now prefer PSRAM with internal fallback; FreeRTOS controls and UART driver buffers remain internal.
- The 5,360-byte transactional user-database candidate now prefers PSRAM with internal fallback while the live database remains internal. Candidate contents are wiped after each transaction and wiped/freed on initialization or recovery failure.
- UART and admin-SSH completion formatter buffers were reduced from 2 KiB to 1 KiB each; current worst-case output is 890 bytes and overflow remains fail-closed.
- Linked RAM fell from 99,508 to 92,188 bytes (7,320 bytes). PSRAM placement of serial payloads additionally removes about 24 KiB of normal internal-heap pressure on the target.
- `pio run` passes. A preliminary target run reports significantly more free memory and stable, improved operation after these changes. This is useful evidence but not completion of Phase 8C validation.
- Remaining targeted checks include stored/migrated/recovered user-database mutations, USB enumeration, HTTPS start/stop failure recovery where injectable, SSH initialization/login, completion display, and sustained multi-transport serial traffic while checking `memory` telemetry.
## Clearly incomplete or transitional areas ## Clearly incomplete or transitional areas
@@ -21,12 +30,10 @@ Based on checked-in source plus `README.md` and `docs/roadmap.md`:
- Browser authentication still uses HTTP Basic; integrated login/logout sessions are planned. - Browser authentication still uses HTTP Basic; integrated login/logout sessions are planned.
- NVS encryption, secure boot/flash encryption review, authentication rate limiting, production certificate/provisioning policy, and OTA are not implemented. - NVS encryption, secure boot/flash encryption review, authentication rate limiting, production certificate/provisioning policy, and OTA are not implemented.
## Known inconsistencies ## Known inconsistencies
These observations should be checked when touching the relevant area; they are not automatically bugs requiring unrelated cleanup. These observations should be checked when touching the relevant area; they are not automatically bugs requiring unrelated cleanup.
- Some source comments still call shared commands UART0-only or call the current local status/control task read-only. - Some source comments still call shared commands UART0-only or call the current local status/control task read-only.
- `USER_DATABASE_LOAD_EMPTY` is only an initialization/failure sentinel at the checked-in revision: every successful `user_database_init()` path returns `STORED` or `MIGRATED_LEGACY`, so `main.c`'s successful "new empty" log branch is unreachable. - `USER_DATABASE_LOAD_EMPTY` is only an initialization/failure sentinel at the checked-in revision: every successful `user_database_init()` path returns `STORED` or `MIGRATED_LEGACY`, so `main.c`'s successful "new empty" log branch is unreachable.
- SSH startup is currently gated on successful `web_security` initialization even though SSH uses separate host-key material. **Needs verification:** whether this coupling is intentional recovery policy or an accidental startup dependency. - SSH startup is currently gated on successful `web_security` initialization even though SSH uses separate host-key material. **Needs verification:** whether this coupling is intentional recovery policy or an accidental startup dependency.
+2 -2
View File
@@ -96,9 +96,9 @@ Only constraints supported by implementation or current project documentation be
**Decision:** Application settings, users, and identities use separate fixed/versioned NVS blobs. Serial, Wi-Fi, and local-UI working edits are RAM-only until explicitly saved. User mutations and HTTPS/SSH identity changes commit directly as part of the operation. Invalid ordinary configuration generally selects RAM defaults without erasing storage; malformed security material fails closed and needs explicit reset. **Decision:** Application settings, users, and identities use separate fixed/versioned NVS blobs. Serial, Wi-Fi, and local-UI working edits are RAM-only until explicitly saved. User mutations and HTTPS/SSH identity changes commit directly as part of the operation. Invalid ordinary configuration generally selects RAM defaults without erasing storage; malformed security material fails closed and needs explicit reset.
**Rationale/evidence:** Serial, Wi-Fi, local UI, web security, users, and SSH security each validate schema/size and own their namespace. User/security mutations build and validate candidate state before committing it; security modules avoid silently replacing an established identity. **Rationale/evidence:** Serial, Wi-Fi, local UI, web security, users, and SSH security each validate schema/size and own their namespace. User/security mutations build and validate candidate state before committing it; security modules avoid silently replacing an established identity. The live user database remains internal while its 5,360-byte candidate is a persistent PSRAM-preferred allocation with internal fallback and is wiped after every transaction.
**Consequence for future changes:** Add schema versions and transactional candidate validation. Do not overwrite unknown records automatically; provide explicit migration/reset behavior. Preserve the distinct persistence contracts: explicit save/load/default/reset for working configuration and per-blob commit-before-live-install for user and identity mutation. Pre-bootstrap legacy credential rotation spans `web_sec/material` and `user_db/database`, is not cross-namespace atomic, and relies on boot reconciliation after interruption. **Consequence for future changes:** Add schema versions and transactional candidate validation. Do not overwrite unknown records automatically; provide explicit migration/reset behavior. Preserve the distinct persistence contracts: explicit save/load/default/reset for working configuration and per-blob commit-before-live-install for user and identity mutation. Keep candidate ownership mutex-local and wipe/free it on initialization or recovery failure. Recheck external-buffer staging in the flash/NVS implementation when upgrading from the pinned ESP-IDF 5.5 baseline. Pre-bootstrap legacy credential rotation spans `web_sec/material` and `user_db/database`, is not cross-namespace atomic, and relies on boot reconciliation after interruption.
**Relevant files:** `src/serial_config.c`, `src/wifi_config.c`, `src/local_ui_config.c`, `src/web_security.c`, `src/user_database.c`, `src/ssh_security.c` **Relevant files:** `src/serial_config.c`, `src/wifi_config.c`, `src/local_ui_config.c`, `src/web_security.c`, `src/user_database.c`, `src/ssh_security.c`
+1 -2
View File
@@ -29,7 +29,6 @@
#define ADMIN_UART_CONSOLE_TASK_PRIORITY 3U #define ADMIN_UART_CONSOLE_TASK_PRIORITY 3U
#define ADMIN_SSH_CONSOLE_MAX_ARGUMENTS 10U #define ADMIN_SSH_CONSOLE_MAX_ARGUMENTS 10U
#define ADMIN_SSH_CONSOLE_HISTORY_DEPTH 4U #define ADMIN_SSH_CONSOLE_HISTORY_DEPTH 4U
#define ADMIN_SSH_CONSOLE_COMPLETION_OUTPUT_CAPACITY 2048U
#define ADMIN_SSH_CONTROL_QUEUE_LENGTH 2U #define ADMIN_SSH_CONTROL_QUEUE_LENGTH 2U
#define ADMIN_SSH_CONTROL_TASK_STACK_SIZE 4096U #define ADMIN_SSH_CONTROL_TASK_STACK_SIZE 4096U
#define ADMIN_SSH_CONTROL_TASK_PRIORITY 3U #define ADMIN_SSH_CONTROL_TASK_PRIORITY 3U
@@ -94,7 +93,7 @@ typedef struct {
static portMUX_TYPE s_lock = portMUX_INITIALIZER_UNLOCKED; static portMUX_TYPE s_lock = portMUX_INITIALIZER_UNLOCKED;
static admin_session_t s_sessions[ADMIN_SSH_CONSOLE_MAX_SESSIONS]; static admin_session_t s_sessions[ADMIN_SSH_CONSOLE_MAX_SESSIONS];
/* admin_ssh_console_feed_input() is called only by the sole SSH owner task. */ /* admin_ssh_console_feed_input() is called only by the sole SSH owner task. */
static char s_completion_output[ADMIN_SSH_CONSOLE_COMPLETION_OUTPUT_CAPACITY]; static char s_completion_output[CONSOLE_COMPLETION_OUTPUT_CAPACITY];
static StaticQueue_t s_request_queue_storage; static StaticQueue_t s_request_queue_storage;
static uint8_t s_request_queue_bytes[ADMIN_SSH_CONSOLE_REQUEST_QUEUE_LENGTH * static uint8_t s_request_queue_bytes[ADMIN_SSH_CONSOLE_REQUEST_QUEUE_LENGTH *
+1 -1
View File
@@ -329,7 +329,7 @@ static ssize_t console_read_with_late_terminal_upgrade(int file_descriptor,
} }
/* The UART frontend is the sole caller of linenoise's completion callback. */ /* The UART frontend is the sole caller of linenoise's completion callback. */
static char s_uart_completion_output[2048U]; static char s_uart_completion_output[CONSOLE_COMPLETION_OUTPUT_CAPACITY];
static void console_completion_callback(const char *buffer, linenoiseCompletions *completions) static void console_completion_callback(const char *buffer, linenoiseCompletions *completions)
{ {
+3
View File
@@ -9,6 +9,9 @@
extern "C" { extern "C" {
#endif #endif
/* Current longest formatted match list is below this; overflow fails closed. */
#define CONSOLE_COMPLETION_OUTPUT_CAPACITY 1024U
/* Install late-terminal upgrade handling and project-specific completion. */ /* Install late-terminal upgrade handling and project-specific completion. */
void console_completion_install(void); void console_completion_install(void);
+31 -4
View File
@@ -7,6 +7,7 @@
#include "driver/gpio.h" #include "driver/gpio.h"
#include "driver/uart.h" #include "driver/uart.h"
#include "esp_check.h" #include "esp_check.h"
#include "esp_heap_caps.h"
#include "esp_log.h" #include "esp_log.h"
#include "freertos/queue.h" #include "freertos/queue.h"
#include "freertos/semphr.h" #include "freertos/semphr.h"
@@ -17,8 +18,10 @@
#define SERIAL_UART_RX_RING_SIZE 8192 #define SERIAL_UART_RX_RING_SIZE 8192
#define SERIAL_UART_EVENT_QUEUE_SIZE 64 #define SERIAL_UART_EVENT_QUEUE_SIZE 64
#define SERIAL_RX_STREAM_SIZE 16384 #define SERIAL_RX_STREAM_SIZE 16384U
#define SERIAL_TX_STREAM_SIZE 8192 #define SERIAL_TX_STREAM_SIZE 8192U
#define SERIAL_RX_STREAM_STORAGE_SIZE (SERIAL_RX_STREAM_SIZE + 1U)
#define SERIAL_TX_STREAM_STORAGE_SIZE (SERIAL_TX_STREAM_SIZE + 1U)
#define SERIAL_IO_CHUNK_SIZE 256 #define SERIAL_IO_CHUNK_SIZE 256
#define SERIAL_TASK_STACK_SIZE 4096 #define SERIAL_TASK_STACK_SIZE 4096
#define SERIAL_TASK_PRIORITY 10 #define SERIAL_TASK_PRIORITY 10
@@ -32,6 +35,10 @@ static SemaphoreHandle_t s_state_mutex;
static SemaphoreHandle_t s_task_stopped; static SemaphoreHandle_t s_task_stopped;
static StreamBufferHandle_t s_rx_stream; static StreamBufferHandle_t s_rx_stream;
static StreamBufferHandle_t s_tx_stream; static StreamBufferHandle_t s_tx_stream;
static StaticStreamBuffer_t s_rx_stream_control;
static StaticStreamBuffer_t s_tx_stream_control;
static uint8_t *s_rx_stream_storage;
static uint8_t *s_tx_stream_storage;
static QueueHandle_t s_uart_event_queue; static QueueHandle_t s_uart_event_queue;
static TaskHandle_t s_event_task; static TaskHandle_t s_event_task;
static portMUX_TYPE s_counter_lock = portMUX_INITIALIZER_UNLOCKED; static portMUX_TYPE s_counter_lock = portMUX_INITIALIZER_UNLOCKED;
@@ -472,8 +479,24 @@ esp_err_t serial_service_init(const serial_config_t *initial_config)
s_state_mutex = xSemaphoreCreateMutex(); s_state_mutex = xSemaphoreCreateMutex();
s_task_stopped = xSemaphoreCreateBinary(); s_task_stopped = xSemaphoreCreateBinary();
s_rx_stream = xStreamBufferCreate(SERIAL_RX_STREAM_SIZE, 1); s_rx_stream_storage = heap_caps_calloc_prefer(
s_tx_stream = xStreamBufferCreate(SERIAL_TX_STREAM_SIZE, 1); 1U, SERIAL_RX_STREAM_STORAGE_SIZE, 2,
MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT,
MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
s_tx_stream_storage = heap_caps_calloc_prefer(
1U, SERIAL_TX_STREAM_STORAGE_SIZE, 2,
MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT,
MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
if (s_rx_stream_storage != NULL) {
s_rx_stream = xStreamBufferCreateStatic(
SERIAL_RX_STREAM_STORAGE_SIZE, 1U, s_rx_stream_storage,
&s_rx_stream_control);
}
if (s_tx_stream_storage != NULL) {
s_tx_stream = xStreamBufferCreateStatic(
SERIAL_TX_STREAM_STORAGE_SIZE, 1U, s_tx_stream_storage,
&s_tx_stream_control);
}
if (s_state_mutex == NULL || s_task_stopped == NULL || if (s_state_mutex == NULL || s_task_stopped == NULL ||
s_rx_stream == NULL || s_tx_stream == NULL) { s_rx_stream == NULL || s_tx_stream == NULL) {
if (s_state_mutex != NULL) { if (s_state_mutex != NULL) {
@@ -488,10 +511,14 @@ esp_err_t serial_service_init(const serial_config_t *initial_config)
if (s_tx_stream != NULL) { if (s_tx_stream != NULL) {
vStreamBufferDelete(s_tx_stream); vStreamBufferDelete(s_tx_stream);
} }
heap_caps_free(s_rx_stream_storage);
heap_caps_free(s_tx_stream_storage);
s_state_mutex = NULL; s_state_mutex = NULL;
s_task_stopped = NULL; s_task_stopped = NULL;
s_rx_stream = NULL; s_rx_stream = NULL;
s_tx_stream = NULL; s_tx_stream = NULL;
s_rx_stream_storage = NULL;
s_tx_stream_storage = NULL;
return ESP_ERR_NO_MEM; return ESP_ERR_NO_MEM;
} }
+15 -3
View File
@@ -89,6 +89,7 @@ static uint32_t s_external_close_id[SSH_TRANSPORT_MAX_SESSIONS];
static ssh_transport_counters_t s_counters; static ssh_transport_counters_t s_counters;
static SemaphoreHandle_t s_command_mutex; static SemaphoreHandle_t s_command_mutex;
static bool s_initializing; static bool s_initializing;
static bool s_init_faulted;
static bool s_initialized; static bool s_initialized;
static bool s_running; static bool s_running;
static bool s_transitioning; static bool s_transitioning;
@@ -1294,20 +1295,29 @@ esp_err_t ssh_transport_init(void)
taskEXIT_CRITICAL(&s_lock); taskEXIT_CRITICAL(&s_lock);
return ESP_OK; return ESP_OK;
} }
if (s_initializing) { if (s_initializing || s_init_faulted) {
taskEXIT_CRITICAL(&s_lock); taskEXIT_CRITICAL(&s_lock);
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
} }
s_initializing = true; s_initializing = true;
taskEXIT_CRITICAL(&s_lock); taskEXIT_CRITICAL(&s_lock);
bool wolfssh_initialized = false;
esp_err_t error = secure_random_init(); esp_err_t error = secure_random_init();
if (error != ESP_OK) { if (error != ESP_OK) {
error = ESP_ERR_INVALID_STATE; error = ESP_ERR_INVALID_STATE;
goto fail; goto fail;
} }
if (wolfSSL_SetAllocators(ssh_malloc, ssh_free, ssh_realloc) != 0 || if (wolfSSL_SetAllocators(ssh_malloc, ssh_free, ssh_realloc) != 0) {
wolfSSH_Init() != WS_SUCCESS || wc_SetSeed_Cb(ssh_seed) != 0) { error = ESP_FAIL;
goto fail;
}
if (wolfSSH_Init() != WS_SUCCESS) {
error = ESP_FAIL;
goto fail;
}
wolfssh_initialized = true;
if (wc_SetSeed_Cb(ssh_seed) != 0) {
error = ESP_FAIL; error = ESP_FAIL;
goto fail; goto fail;
} }
@@ -1347,7 +1357,9 @@ esp_err_t ssh_transport_init(void)
return ESP_OK; return ESP_OK;
fail: fail:
bool cleanup_failed = wolfssh_initialized && wolfSSH_Cleanup() != WS_SUCCESS;
taskENTER_CRITICAL(&s_lock); taskENTER_CRITICAL(&s_lock);
s_init_faulted = cleanup_failed;
s_initializing = false; s_initializing = false;
taskEXIT_CRITICAL(&s_lock); taskEXIT_CRITICAL(&s_lock);
return error; return error;
+30 -16
View File
@@ -47,6 +47,7 @@ static portMUX_TYPE s_state_lock = portMUX_INITIALIZER_UNLOCKED;
static atomic_bool s_initialized; static atomic_bool s_initialized;
static atomic_bool s_initializing; static atomic_bool s_initializing;
static atomic_bool s_init_faulted;
static atomic_uint s_usb_state; static atomic_uint s_usb_state;
/* Changes on every effective CDC open/close boundary, even during one task poll. */ /* Changes on every effective CDC open/close boundary, even during one task poll. */
static atomic_uint s_connection_generation; static atomic_uint s_connection_generation;
@@ -816,13 +817,19 @@ static void reset_uninitialized_state(void)
taskEXIT_CRITICAL(&s_state_lock); taskEXIT_CRITICAL(&s_state_lock);
} }
static void cleanup_init_allocations(bool cdc_initialized, bool driver_installed) static esp_err_t cleanup_init_allocations(bool cdc_initialized, bool driver_installed)
{ {
if (cdc_initialized) { if (cdc_initialized) {
(void)tinyusb_cdcacm_deinit(TINYUSB_CDC_ACM_0); esp_err_t error = tinyusb_cdcacm_deinit(TINYUSB_CDC_ACM_0);
if (error != ESP_OK) {
return error;
}
} }
if (driver_installed) { if (driver_installed) {
(void)tinyusb_driver_uninstall(); esp_err_t error = tinyusb_driver_uninstall();
if (error != ESP_OK) {
return error;
}
} }
if (s_control_queue != NULL) { if (s_control_queue != NULL) {
vQueueDelete(s_control_queue); vQueueDelete(s_control_queue);
@@ -833,12 +840,27 @@ static void cleanup_init_allocations(bool cdc_initialized, bool driver_installed
s_host_rx_stream = NULL; s_host_rx_stream = NULL;
} }
reset_uninitialized_state(); reset_uninitialized_state();
return ESP_OK;
}
static esp_err_t fail_initialization(esp_err_t original_error,
bool cdc_initialized,
bool driver_installed)
{
esp_err_t cleanup_error = cleanup_init_allocations(cdc_initialized, driver_installed);
if (cleanup_error != ESP_OK) {
/* Retain callback-facing storage and reject retries after incomplete teardown. */
atomic_store(&s_init_faulted, true);
original_error = cleanup_error;
}
atomic_store(&s_initializing, false);
return original_error;
} }
esp_err_t usb_cdc_transport_init(void) esp_err_t usb_cdc_transport_init(void)
{ {
bool expected = false; bool expected = false;
if (atomic_load(&s_initialized) || if (atomic_load(&s_initialized) || atomic_load(&s_init_faulted) ||
!atomic_compare_exchange_strong(&s_initializing, &expected, true)) { !atomic_compare_exchange_strong(&s_initializing, &expected, true)) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
} }
@@ -870,9 +892,7 @@ esp_err_t usb_cdc_transport_init(void)
s_control_queue = xQueueCreate(USB_CDC_CONTROL_QUEUE_LENGTH, s_control_queue = xQueueCreate(USB_CDC_CONTROL_QUEUE_LENGTH,
sizeof(usb_cdc_control_t)); sizeof(usb_cdc_control_t));
if (s_control_queue == NULL) { if (s_control_queue == NULL) {
cleanup_init_allocations(false, false); return fail_initialization(ESP_ERR_NO_MEM, false, false);
atomic_store(&s_initializing, false);
return ESP_ERR_NO_MEM;
} }
/* ESP32-S3's default full-speed internal PHY is fixed to GPIO19/20. */ /* ESP32-S3's default full-speed internal PHY is fixed to GPIO19/20. */
@@ -884,9 +904,7 @@ esp_err_t usb_cdc_transport_init(void)
result = tinyusb_driver_install(&usb_config); result = tinyusb_driver_install(&usb_config);
if (result != ESP_OK) { if (result != ESP_OK) {
cleanup_init_allocations(false, false); return fail_initialization(result, false, false);
atomic_store(&s_initializing, false);
return result;
} }
const tinyusb_config_cdcacm_t cdc_config = { const tinyusb_config_cdcacm_t cdc_config = {
@@ -898,9 +916,7 @@ esp_err_t usb_cdc_transport_init(void)
}; };
result = tinyusb_cdcacm_init(&cdc_config); result = tinyusb_cdcacm_init(&cdc_config);
if (result != ESP_OK) { if (result != ESP_OK) {
cleanup_init_allocations(false, true); return fail_initialization(result, false, true);
atomic_store(&s_initializing, false);
return result;
} }
TaskHandle_t task = NULL; TaskHandle_t task = NULL;
@@ -910,9 +926,7 @@ esp_err_t usb_cdc_transport_init(void)
NULL, NULL,
USB_CDC_TASK_PRIORITY, USB_CDC_TASK_PRIORITY,
&task) != pdPASS) { &task) != pdPASS) {
cleanup_init_allocations(true, true); return fail_initialization(ESP_ERR_NO_MEM, true, true);
atomic_store(&s_initializing, false);
return ESP_ERR_NO_MEM;
} }
atomic_store(&s_transport_task, (uintptr_t)task); atomic_store(&s_transport_task, (uintptr_t)task);
+65 -33
View File
@@ -6,6 +6,7 @@
#include <stddef.h> #include <stddef.h>
#include <string.h> #include <string.h>
#include "esp_heap_caps.h"
#include "freertos/FreeRTOS.h" #include "freertos/FreeRTOS.h"
#include "freertos/semphr.h" #include "freertos/semphr.h"
#include "mbedtls/ecp.h" #include "mbedtls/ecp.h"
@@ -66,7 +67,7 @@ typedef struct {
static SemaphoreHandle_t s_mutex; static SemaphoreHandle_t s_mutex;
static stored_database_t s_database; static stored_database_t s_database;
static stored_database_t s_candidate; static stored_database_t *s_candidate;
static bool s_initialized; static bool s_initialized;
static uint8_t s_dummy_salt[USER_DATABASE_PASSWORD_SALT_LENGTH]; static uint8_t s_dummy_salt[USER_DATABASE_PASSWORD_SALT_LENGTH];
static uint8_t s_dummy_hash[USER_DATABASE_PASSWORD_HASH_LENGTH]; static uint8_t s_dummy_hash[USER_DATABASE_PASSWORD_HASH_LENGTH];
@@ -80,9 +81,30 @@ static esp_err_t initialize_dummy_verifier(void)
return error; return error;
} }
static esp_err_t allocate_candidate(void)
{
if (s_candidate != NULL) {
return ESP_ERR_INVALID_STATE;
}
s_candidate = heap_caps_calloc_prefer(
1U, sizeof(*s_candidate), 2,
MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT,
MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
return s_candidate != NULL ? ESP_OK : ESP_ERR_NO_MEM;
}
static void discard_candidate(void) static void discard_candidate(void)
{ {
secure_wipe(&s_candidate, sizeof(s_candidate)); if (s_candidate != NULL) {
secure_wipe(s_candidate, sizeof(*s_candidate));
}
}
static void release_candidate(void)
{
discard_candidate();
heap_caps_free(s_candidate);
s_candidate = NULL;
} }
static bool constant_time_equal(const uint8_t *left, const uint8_t *right, size_t length) static bool constant_time_equal(const uint8_t *left, const uint8_t *right, size_t length)
@@ -437,10 +459,10 @@ static esp_err_t next_generation(uint32_t *generation)
static esp_err_t commit_candidate_locked(void) static esp_err_t commit_candidate_locked(void)
{ {
recount(&s_candidate); recount(s_candidate);
esp_err_t error = next_generation(&s_candidate.generation); esp_err_t error = next_generation(&s_candidate->generation);
if (error == ESP_OK) { if (error == ESP_OK) {
error = validate_database(&s_candidate); error = validate_database(s_candidate);
} }
if (error != ESP_OK) { if (error != ESP_OK) {
discard_candidate(); discard_candidate();
@@ -451,7 +473,7 @@ static esp_err_t commit_candidate_locked(void)
error = nvs_open(USER_DATABASE_NVS_NAMESPACE, NVS_READWRITE, &handle); error = nvs_open(USER_DATABASE_NVS_NAMESPACE, NVS_READWRITE, &handle);
if (error == ESP_OK) { if (error == ESP_OK) {
error = nvs_set_blob(handle, USER_DATABASE_NVS_BLOB_KEY, error = nvs_set_blob(handle, USER_DATABASE_NVS_BLOB_KEY,
&s_candidate, sizeof(s_candidate)); s_candidate, sizeof(*s_candidate));
if (error == ESP_OK) { if (error == ESP_OK) {
error = nvs_commit(handle); error = nvs_commit(handle);
} }
@@ -459,7 +481,7 @@ static esp_err_t commit_candidate_locked(void)
} }
if (error == ESP_OK) { if (error == ESP_OK) {
secure_wipe(&s_database, sizeof(s_database)); secure_wipe(&s_database, sizeof(s_database));
s_database = s_candidate; s_database = *s_candidate;
} }
discard_candidate(); discard_candidate();
return error; return error;
@@ -530,8 +552,8 @@ static esp_err_t synchronize_legacy_locked(
return error; return error;
} }
s_candidate = s_database; *s_candidate = s_database;
stored_user_t *candidate_user = &s_candidate.users[index]; stored_user_t *candidate_user = &s_candidate->users[index];
error = set_record_password(candidate_user, legacy->password, error = set_record_password(candidate_user, legacy->password,
legacy->password_length); legacy->password_length);
if (error == ESP_OK) { if (error == ESP_OK) {
@@ -557,12 +579,16 @@ esp_err_t user_database_init(const user_database_legacy_credentials_t *legacy,
if (s_mutex == NULL) { if (s_mutex == NULL) {
return ESP_ERR_NO_MEM; return ESP_ERR_NO_MEM;
} }
esp_err_t error = nvs_flash_init(); esp_err_t error = allocate_candidate();
if (error != ESP_OK) { if (error != ESP_OK) {
vSemaphoreDelete(s_mutex); vSemaphoreDelete(s_mutex);
s_mutex = NULL; s_mutex = NULL;
return error; return error;
} }
error = nvs_flash_init();
if (error != ESP_OK) {
goto init_failed;
}
bool storage_missing = false; bool storage_missing = false;
nvs_handle_t handle; nvs_handle_t handle;
@@ -621,7 +647,7 @@ esp_err_t user_database_init(const user_database_legacy_credentials_t *legacy,
} }
*load_result = USER_DATABASE_LOAD_MIGRATED_LEGACY; *load_result = USER_DATABASE_LOAD_MIGRATED_LEGACY;
recount(&s_database); recount(&s_database);
s_candidate = s_database; *s_candidate = s_database;
error = commit_candidate_locked(); error = commit_candidate_locked();
if (error != ESP_OK) { if (error != ESP_OK) {
goto init_failed; goto init_failed;
@@ -635,7 +661,7 @@ esp_err_t user_database_init(const user_database_legacy_credentials_t *legacy,
init_failed: init_failed:
secure_wipe(&s_database, sizeof(s_database)); secure_wipe(&s_database, sizeof(s_database));
discard_candidate(); release_candidate();
secure_wipe(s_dummy_salt, sizeof(s_dummy_salt)); secure_wipe(s_dummy_salt, sizeof(s_dummy_salt));
secure_wipe(s_dummy_hash, sizeof(s_dummy_hash)); secure_wipe(s_dummy_hash, sizeof(s_dummy_hash));
vSemaphoreDelete(s_mutex); vSemaphoreDelete(s_mutex);
@@ -673,17 +699,23 @@ esp_err_t user_database_recover_from_legacy(
if (s_mutex == NULL) { if (s_mutex == NULL) {
return ESP_ERR_NO_MEM; return ESP_ERR_NO_MEM;
} }
esp_err_t error = nvs_flash_init(); esp_err_t error = allocate_candidate();
if (error != ESP_OK) {
vSemaphoreDelete(s_mutex);
s_mutex = NULL;
return error;
}
error = nvs_flash_init();
if (error != ESP_OK) { if (error != ESP_OK) {
goto recovery_failed; goto recovery_failed;
} }
memset(&s_database, 0, sizeof(s_database)); memset(&s_database, 0, sizeof(s_database));
memset(&s_candidate, 0, sizeof(s_candidate)); memset(s_candidate, 0, sizeof(*s_candidate));
s_candidate.version = USER_DATABASE_SCHEMA_VERSION; s_candidate->version = USER_DATABASE_SCHEMA_VERSION;
s_candidate.size = sizeof(s_candidate); s_candidate->size = sizeof(*s_candidate);
s_candidate.generation = 1U; s_candidate->generation = 1U;
error = initialize_user(&s_candidate.users[0], legacy->username, error = initialize_user(&s_candidate->users[0], legacy->username,
legacy->username_length, USER_ROLE_USER, legacy->username_length, USER_ROLE_USER,
legacy->password, legacy->password_length); legacy->password, legacy->password_length);
if (error == ESP_OK) { if (error == ESP_OK) {
@@ -701,7 +733,7 @@ esp_err_t user_database_recover_from_legacy(
recovery_failed: recovery_failed:
secure_wipe(&s_database, sizeof(s_database)); secure_wipe(&s_database, sizeof(s_database));
discard_candidate(); release_candidate();
secure_wipe(s_dummy_salt, sizeof(s_dummy_salt)); secure_wipe(s_dummy_salt, sizeof(s_dummy_salt));
secure_wipe(s_dummy_hash, sizeof(s_dummy_hash)); secure_wipe(s_dummy_hash, sizeof(s_dummy_hash));
vSemaphoreDelete(s_mutex); vSemaphoreDelete(s_mutex);
@@ -918,11 +950,11 @@ static esp_err_t create_locked(const uint8_t *username, size_t username_length,
if (free_index < 0) { if (free_index < 0) {
return ESP_ERR_NO_MEM; return ESP_ERR_NO_MEM;
} }
s_candidate = s_database; *s_candidate = s_database;
esp_err_t error = initialize_user(&s_candidate.users[free_index], username, esp_err_t error = initialize_user(&s_candidate->users[free_index], username,
username_length, role, password, password_length); username_length, role, password, password_length);
if (error == ESP_OK && role == USER_ROLE_ADMIN) { if (error == ESP_OK && role == USER_ROLE_ADMIN) {
s_candidate.admin_bootstrapped = 1U; s_candidate->admin_bootstrapped = 1U;
} }
if (error == ESP_OK) { if (error == ESP_OK) {
return commit_candidate_locked(); return commit_candidate_locked();
@@ -984,15 +1016,15 @@ esp_err_t user_database_bootstrap_admin(const uint8_t *password,
error = create_locked(s_admin_username, sizeof(s_admin_username) - 1U, error = create_locked(s_admin_username, sizeof(s_admin_username) - 1U,
USER_ROLE_ADMIN, password, password_length); USER_ROLE_ADMIN, password, password_length);
} else { } else {
s_candidate = s_database; *s_candidate = s_database;
stored_user_t *user = &s_candidate.users[index]; stored_user_t *user = &s_candidate->users[index];
error = set_record_password(user, password, password_length); error = set_record_password(user, password, password_length);
if (error == ESP_OK) { if (error == ESP_OK) {
user->role = USER_ROLE_ADMIN; user->role = USER_ROLE_ADMIN;
error = next_generation(&user->auth_generation); error = next_generation(&user->auth_generation);
} }
if (error == ESP_OK) { if (error == ESP_OK) {
s_candidate.admin_bootstrapped = 1U; s_candidate->admin_bootstrapped = 1U;
error = commit_candidate_locked(); error = commit_candidate_locked();
} else { } else {
discard_candidate(); discard_candidate();
@@ -1026,7 +1058,7 @@ static esp_err_t mutate_user_begin(const uint8_t *username, size_t username_leng
if (*index < 0) { if (*index < 0) {
return ESP_ERR_NOT_FOUND; return ESP_ERR_NOT_FOUND;
} }
s_candidate = s_database; *s_candidate = s_database;
return ESP_OK; return ESP_OK;
} }
@@ -1050,7 +1082,7 @@ esp_err_t user_database_delete(const uint8_t *username, size_t username_length)
error = ESP_ERR_INVALID_STATE; error = ESP_ERR_INVALID_STATE;
discard_candidate(); discard_candidate();
} else { } else {
secure_wipe(&s_candidate.users[index], sizeof(s_candidate.users[index])); secure_wipe(&s_candidate->users[index], sizeof(s_candidate->users[index]));
error = commit_candidate_locked(); error = commit_candidate_locked();
} }
} }
@@ -1069,7 +1101,7 @@ esp_err_t user_database_set_role(const uint8_t *username, size_t username_length
int index; int index;
esp_err_t error = mutate_user_begin(username, username_length, &index); esp_err_t error = mutate_user_begin(username, username_length, &index);
if (error == ESP_OK) { if (error == ESP_OK) {
stored_user_t *user = &s_candidate.users[index]; stored_user_t *user = &s_candidate->users[index];
if (user->role == role) { if (user->role == role) {
error = ESP_OK; error = ESP_OK;
discard_candidate(); discard_candidate();
@@ -1080,7 +1112,7 @@ esp_err_t user_database_set_role(const uint8_t *username, size_t username_length
user->role = (uint8_t)role; user->role = (uint8_t)role;
error = next_generation(&user->auth_generation); error = next_generation(&user->auth_generation);
if (error == ESP_OK && role == USER_ROLE_ADMIN) { if (error == ESP_OK && role == USER_ROLE_ADMIN) {
s_candidate.admin_bootstrapped = 1U; s_candidate->admin_bootstrapped = 1U;
} }
if (error == ESP_OK) { if (error == ESP_OK) {
error = commit_candidate_locked(); error = commit_candidate_locked();
@@ -1104,7 +1136,7 @@ esp_err_t user_database_set_password(const uint8_t *username, size_t username_le
int index; int index;
esp_err_t error = mutate_user_begin(username, username_length, &index); esp_err_t error = mutate_user_begin(username, username_length, &index);
if (error == ESP_OK) { if (error == ESP_OK) {
stored_user_t *user = &s_candidate.users[index]; stored_user_t *user = &s_candidate->users[index];
error = set_record_password(user, password, password_length); error = set_record_password(user, password, password_length);
if (error == ESP_OK) { if (error == ESP_OK) {
error = next_generation(&user->auth_generation); error = next_generation(&user->auth_generation);
@@ -1149,7 +1181,7 @@ esp_err_t user_database_add_ssh_key(
int user_index; int user_index;
esp_err_t error = mutate_user_begin(username, username_length, &user_index); esp_err_t error = mutate_user_begin(username, username_length, &user_index);
if (error == ESP_OK) { if (error == ESP_OK) {
stored_user_t *user = &s_candidate.users[user_index]; stored_user_t *user = &s_candidate->users[user_index];
int free_index = -1; int free_index = -1;
for (size_t index = 0U; for (size_t index = 0U;
index < USER_DATABASE_MAX_SSH_KEYS_PER_USER; ++index) { index < USER_DATABASE_MAX_SSH_KEYS_PER_USER; ++index) {
@@ -1210,7 +1242,7 @@ esp_err_t user_database_remove_ssh_key(const uint8_t *username,
int user_index; int user_index;
esp_err_t error = mutate_user_begin(username, username_length, &user_index); esp_err_t error = mutate_user_begin(username, username_length, &user_index);
if (error == ESP_OK) { if (error == ESP_OK) {
stored_user_t *user = &s_candidate.users[user_index]; stored_user_t *user = &s_candidate->users[user_index];
if (user->keys[key_index].active == 0U) { if (user->keys[key_index].active == 0U) {
error = ESP_ERR_NOT_FOUND; error = ESP_ERR_NOT_FOUND;
discard_candidate(); discard_candidate();
@@ -1239,7 +1271,7 @@ esp_err_t user_database_clear_ssh_keys(const uint8_t *username,
int user_index; int user_index;
esp_err_t error = mutate_user_begin(username, username_length, &user_index); esp_err_t error = mutate_user_begin(username, username_length, &user_index);
if (error == ESP_OK) { if (error == ESP_OK) {
stored_user_t *user = &s_candidate.users[user_index]; stored_user_t *user = &s_candidate->users[user_index];
if (user->key_count == 0U) { if (user->key_count == 0U) {
error = ESP_OK; error = ESP_OK;
discard_candidate(); discard_candidate();
+9 -2
View File
@@ -695,8 +695,13 @@ esp_err_t web_server_start(void)
serial_transport_attached = attach_error == ESP_OK; serial_transport_attached = attach_error == ESP_OK;
} }
if (error != ESP_OK && server != NULL) { if (error != ESP_OK && server != NULL) {
(void)httpd_ssl_stop(server); esp_err_t cleanup_error = httpd_ssl_stop(server);
server = NULL; if (cleanup_error == ESP_OK) {
server = NULL;
} else {
/* Retain ownership so stop can retry and start cannot allocate a second server. */
error = cleanup_error;
}
} }
xSemaphoreTake(s_server_mutex, portMAX_DELAY); xSemaphoreTake(s_server_mutex, portMAX_DELAY);
@@ -708,6 +713,8 @@ esp_err_t web_server_start(void)
s_server = server; s_server = server;
++s_counters.starts; ++s_counters.starts;
} else { } else {
/* A non-NULL handle is a partially started server whose cleanup failed. */
s_server = server;
++s_counters.start_failures; ++s_counters.start_failures;
} }
xSemaphoreGive(s_server_mutex); xSemaphoreGive(s_server_mutex);