What changed, and why it matters
This commit is a major internal rewrite of how Blockstream Jade handles USB storage and firmware updates over USB. It simplifies the code, removes several old synchronization mechanisms, and adds timeouts and better error handling. The changes appear to be defensive hardening and reliability fixes rather than a patch for a known active security flaw. There is no mention of a CVE, security advisory, or external researcher in the commit itself.
Treat this as a reliability and hardening update. Review the new single-mutex USB state machine for potential deadlocks or missed events during device detach, and verify the OTA timeout behavior does not introduce premature failures on slow USB drives. No immediate security response is indicated by the commit alone.
Security signals we found
Refactored USB MSC state machine to reduce synchronization primitives and potential race conditions
Added timeout for ota_data message replies to prevent indefinite blocking
Added JADE_LOGE logging for invalid or non-ota_data messages in OTA handler
Removed UI-based USB logging support to avoid freezing when JTAG and logging are both enabled
Added assertions that CBOR-encoded OTA message sizes fit in allocated stack buffers
Changed OTA message IDs from fixed '0' to incrementing integers
Simplified caller API to start()/stop() and moved mount/unmount logic internally
Evidence from the diff
The commit refactors the USB Mass Storage Class (MSC) host driver integration in Jade. It replaces a multi-semaphore, multi-task state machine with a single mutex, single event group, and a queue for MSC events. It removes the old callback-based mount/unmount API in favor of an event-group-based start/stop API. It also hardens OTA over USB: it adds timeouts waiting for ota_data replies, uses distinct message IDs, reduces stack buffer sizes with assertions, logs invalid ota_data messages, and prevents the UI from freezing when JTAG and logging are both enabled. The changes are broad and touch core USB/OTA paths, but the diff shows cleanup and hardening rather than a targeted fix for a specific vulnerability.
Changed components
main/usbhmsc/usbhmsc.cmain/usbhmsc/usbhmsc.hmain/usbhmsc/usbmode.cmain/process/ota_util.cInspect captured patch +317 / −411
diff --git a/main/process/ota_util.c b/main/process/ota_util.c
index ac56cbd..b45f0c6 100644
--- a/main/process/ota_util.c
+++ b/main/process/ota_util.c
@@ -4,7 +4,9 @@
#include "../jade_assert.h"
#include "../jade_wally_verify.h"
#include "../qrmode.h"
+#include "../utils/malloc_ext.h"
#include "ota_defines.h"
+#include "process_utils.h"
#include <ctype.h>
#include <esp_efuse.h>
@@ -110,6 +112,7 @@ void handle_in_bin_data(void* ctx, uint8_t* data, const size_t rawsize)
}
if (!rpc_is_method(&value, "ota_data")) {
+ JADE_LOGE("handle_in_bin_data: message is not ota_data");
joctx->ota_return_status = OTA_ERR_BADDATA;
return;
}
@@ -120,6 +123,7 @@ void handle_in_bin_data(void* ctx, uint8_t* data, const size_t rawsize)
rpc_get_bytes_ptr("params", &value, &inbound_buf, &written);
if (written == 0 || data[0] != joctx->expected_source || written > JADE_OTA_BUF_SIZE || !inbound_buf) {
+ JADE_LOGE("handle_in_bin_data: invalid written or source");
joctx->ota_return_status = OTA_ERR_BADDATA;
return;
}
@@ -148,17 +152,16 @@ void handle_in_bin_data(void* ctx, uint8_t* data, const size_t rawsize)
joctx->remaining_compressed -= written;
- JADE_LOGI("Received ota_data msg %s, payload size %u", joctx->id, written);
-
- JADE_LOGI("compressed: total = %u, current = %u", joctx->compressedsize,
- joctx->compressedsize - joctx->remaining_compressed);
- JADE_LOGI("uncompressed: total = %u, current = %u", joctx->uncompressedsize,
- joctx->uncompressedsize - joctx->remaining_uncompressed);
-
// Send ack after all processing - see comment above.
- uint8_t reply_msg[64];
- jade_process_reply_to_message_result_with_id(
- joctx->id, reply_msg, sizeof(reply_msg), joctx->expected_source, joctx, reply_ok);
+ {
+ uint8_t reply_msg[64];
+ jade_process_reply_to_message_result_with_id(
+ joctx->id, reply_msg, sizeof(reply_msg), joctx->expected_source, joctx, reply_ok);
+ }
+
+ JADE_LOGI("sent ok for ota_data %s(%u), %u/%u->%u/%u", joctx->id, written,
+ joctx->compressedsize - joctx->remaining_compressed, joctx->compressedsize,
+ joctx->uncompressedsize - joctx->remaining_uncompressed, joctx->uncompressedsize);
// Blank out the current msg id once 'ok' is sent for it
joctx->id[0] = '\0';
@@ -280,6 +283,7 @@ cleanup:
if (errmsg) {
JADE_LOGE("%s", errmsg);
jade_process_reject_message(process, errcode, errmsg);
+ joctx = NULL;
}
return joctx;
}
diff --git a/main/usbhmsc/usbhmsc.c b/main/usbhmsc/usbhmsc.c
index 07c8707..38a987e 100644
--- a/main/usbhmsc/usbhmsc.c
+++ b/main/usbhmsc/usbhmsc.c
@@ -4,8 +4,6 @@
#include <esp_err.h>
#include <esp_vfs.h>
#include <esp_vfs_fat.h>
-#include <freertos/FreeRTOS.h>
-#include <freertos/event_groups.h>
#include <freertos/task.h>
#include <msc_host.h>
#include <msc_host_vfs.h>
@@ -17,331 +15,294 @@
#include "../jade_tasks.h"
#include "../power.h"
-#define JADE_ERROR_CHECK(x) \
- do { \
- esp_err_t err = (x); \
- JADE_ASSERT(err == ESP_OK); \
- } while (0);
-
-#define JADE_RETURN_CHECK(x) \
- do { \
- esp_err_t err = (x); \
- if (err != ESP_OK) { \
- JADE_LOGE("JADE_RETURN_CHECK %d line: %d", err, __LINE__); \
- JADE_SEMAPHORE_GIVE(interface_mutex); \
- return false; \
- } \
- } while (0);
-
typedef enum {
- HOST_NO_CLIENT = 0x1,
- HOST_ALL_FREE = 0x2,
- DEVICE_CONNECTED = 0x4,
- DEVICE_DISCONNECTED = 0x8,
- DEVICE_ADDRESS_MASK = 0xFF0,
-} app_event_t;
-
-static SemaphoreHandle_t main_task_semaphore = NULL;
-static SemaphoreHandle_t aux_task_semaphore = NULL;
-static SemaphoreHandle_t interface_mutex = NULL;
-static SemaphoreHandle_t callback_mutex = NULL;
-static TaskHandle_t main_task = NULL;
-static bool volatile usb_device_installed = false;
-
-static bool volatile usbstorage_is_enabled = false;
-static bool volatile usbstorage_is_enabled_subtask = false;
-static EventGroupHandle_t usb_flags;
-
-static msc_host_device_handle_t msc_device = NULL;
-static msc_host_vfs_handle_t vfs_handle = NULL;
-
-static const esp_vfs_fat_mount_config_t mount_config = { .format_if_mount_failed = false, .max_files = 1 };
-
-static usbstorage_callback_t registered_callback = NULL;
-static void* callback_ctx = NULL;
-
-void usbstorage_register_callback(usbstorage_callback_t callback, void* ctx)
+ USBSTATE_NONE = 0x00, // USB task is not started/initialized yet
+ USBSTATE_USB_INSTALLED = 0x01, // usb_host_install() called
+ USBSTATE_MSC_INSTALLED = 0x02, // msc_host_install() called
+ USBSTATE_WORKER_SHUTDOWN = 0x20, // USB clients disconnected, worker task has shutdown
+ USBSTATE_SHUTDOWN_REQUESTED = 0x40, // USB task asked to shutdown
+ USBSTATE_ERROR = 0x80, // An error occurred
+ USBSTATE_TERMINAL_EVENT = USBSTATE_SHUTDOWN_REQUESTED | USBSTATE_ERROR,
+} usbstorage_state_t;
+
+static SemaphoreHandle_t usbstorage_mutex = NULL;
+static TaskHandle_t usbstorage_task = NULL;
+static EventGroupHandle_t usbstorage_flags = NULL;
+static QueueHandle_t usbstorage_msc_queue = NULL;
+static usbstorage_state_t usbstorage_state = USBSTATE_NONE;
+// Disable logging when switching from USB serial to USB storage
+#define USBSTORAGE_DISABLE_LOGGING
+#ifdef USBSTORAGE_DISABLE_LOGGING
+static esp_log_level_t initial_log_level;
+#endif
+
+static usbstorage_state_t usbstorage_state_get(void)
{
- JADE_ASSERT(callback_mutex);
- JADE_SEMAPHORE_TAKE(callback_mutex);
- registered_callback = callback;
- callback_ctx = ctx;
- JADE_SEMAPHORE_GIVE(callback_mutex);
+ usbstorage_state_t state;
+ JADE_SEMAPHORE_TAKE(usbstorage_mutex);
+ state = usbstorage_state;
+ JADE_SEMAPHORE_GIVE(usbstorage_mutex);
+ return state;
}
-static void trigger_event(usbstorage_event_t event, uint8_t device_address)
+static void usbstorage_state_set(const usbstorage_state_t state)
{
- JADE_ASSERT(callback_mutex);
- JADE_SEMAPHORE_TAKE(callback_mutex);
- usbstorage_callback_t callback = registered_callback;
- void* ctx = callback_ctx;
- JADE_SEMAPHORE_GIVE(callback_mutex);
- if (callback != NULL) {
- callback(event, device_address, ctx);
- }
+ JADE_SEMAPHORE_TAKE(usbstorage_mutex);
+ usbstorage_state |= state;
+ JADE_SEMAPHORE_GIVE(usbstorage_mutex);
}
-static void msc_event_cb(const msc_host_event_t* event, void* arg)
+static void msc_event_cb(const msc_host_event_t* event, void* ignore)
{
if (event->event == MSC_DEVICE_CONNECTED) {
- xEventGroupSetBits(usb_flags, DEVICE_CONNECTED | (event->device.address << 4));
+ JADE_LOGI("DEVICE_CONNECTED %d", (int)event->device.address);
} else if (event->event == MSC_DEVICE_DISCONNECTED) {
- xEventGroupSetBits(usb_flags, DEVICE_DISCONNECTED);
- }
-}
-
-static void usb_host_lib_events(const uint32_t timeout)
-{
- uint32_t event_flags;
- const esp_err_t err = usb_host_lib_handle_events(timeout, &event_flags);
-
- if (err == ESP_ERR_TIMEOUT) {
+ JADE_LOGI("DEVICE_DISCONNECTED");
+ } else {
return;
}
-
- JADE_ERROR_CHECK(err);
-
- EventBits_t event = 0;
- if (event_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
- event |= HOST_NO_CLIENT;
- }
- if (event_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
- event |= HOST_ALL_FREE;
- }
- if (event) {
- xEventGroupSetBits(usb_flags, event);
- }
+ xQueueSend(usbstorage_msc_queue, event, portMAX_DELAY);
}
-static void handle_usb_events(void* args)
+static void usbstorage_worker_impl(void* ignore)
{
+ // Loop polling for USB events to process
while (true) {
- usb_host_lib_events(50 / portTICK_PERIOD_MS);
-
- if (!usbstorage_is_enabled_subtask) {
- break;
+ uint32_t usb_flags = 0;
+ usb_host_lib_handle_events(portMAX_DELAY, &usb_flags);
+ if (usb_flags & USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS) {
+ const esp_err_t err = usb_host_device_free_all();
+ if (err == ESP_OK) {
+ // all devices freed already. Fall through to exit below.
+ JADE_LOGI("usbstorage worker: NO_CLIENTS");
+ usb_flags |= USB_HOST_LIB_EVENT_FLAGS_ALL_FREE;
+ } else if (err == ESP_ERR_INVALID_STATE) {
+ JADE_ASSERT_MSG(false, "usb_host_device_free_all returned ESP_ERR_INVALID_STATE");
+ } else if (err != ESP_ERR_NOT_FINISHED) {
+ JADE_LOGW("usb_host_device_free_all returned %d", err);
+ }
}
-
- msc_host_handle_events(50 / portTICK_PERIOD_MS);
-
- if (!usbstorage_is_enabled_subtask) {
+ if (usb_flags & USB_HOST_LIB_EVENT_FLAGS_ALL_FREE) {
+ JADE_LOGI("usbstorage worker: ALL_FREE");
break;
}
}
-
- // msc_host_uninstall will cause the USB_HOST_LIB_EVENT_FLAGS_NO_CLIENTS event
- // so lets clear any last events
- uint32_t event_flags;
- usb_host_lib_handle_events(1, &event_flags);
-
- JADE_ASSERT(!usbstorage_is_enabled);
-
- xSemaphoreGive(aux_task_semaphore);
- for (;;) {
- vTaskDelay(portMAX_DELAY);
- }
+ usbstorage_state_set(USBSTATE_WORKER_SHUTDOWN);
+ vTaskDelete(NULL);
}
-static void usbstorage_task(void* ignore)
+static void usbstorage_impl(void* ignore)
{
+ msc_host_device_handle_t msc_device = NULL;
+ msc_host_vfs_handle_t vfs_handle = NULL;
+ esp_err_t err;
- const usb_host_config_t host_config = { .intr_flags = ESP_INTR_FLAG_LEVEL1 };
- if (usb_host_install(&host_config) != ESP_OK) {
- usbstorage_is_enabled = false;
+ JADE_LOGI("enable_usb_host...");
+ enable_usb_host();
- /* disable_usb_host(); */
- xSemaphoreGive(main_task_semaphore);
- main_task = NULL;
- vTaskDelete(NULL);
- return;
+ JADE_LOGI("usb_host_install...");
+ {
+ const usb_host_config_t config = { .intr_flags = ESP_INTR_FLAG_LEVEL1 };
+ err = usb_host_install(&config);
+ }
+ JADE_LOGD("usb_host_install returned %d", err);
+ usbstorage_state_set(err == ESP_OK ? USBSTATE_USB_INSTALLED : USBSTATE_ERROR);
+ if (err != ESP_OK) {
+ goto cleanup;
}
- usbstorage_is_enabled_subtask = true;
- TaskHandle_t aux_task = NULL;
-
- usb_flags = xEventGroupCreate();
- JADE_ASSERT(usb_flags);
-
- const BaseType_t task_created = xTaskCreatePinnedToCore(
- handle_usb_events, "usb_events", 1024, NULL, JADE_TASK_PRIO_USB, &aux_task, JADE_CORE_SECONDARY);
- JADE_ASSERT(task_created == pdPASS);
- JADE_ASSERT(aux_task);
-
- const msc_host_driver_config_t msc_config = {
- .create_backround_task = false,
- .callback = msc_event_cb,
- };
+ JADE_LOGI("start usb worker...");
+ TaskHandle_t worker_task = NULL;
+ const BaseType_t retval = xTaskCreatePinnedToCore(
+ usbstorage_worker_impl, "usb_worker", 2 * 1024, NULL, JADE_TASK_PRIO_USB, &worker_task, JADE_CORE_SECONDARY);
+ JADE_ASSERT(retval == pdPASS && worker_task);
+ JADE_LOGI("msc_host_install..");
{
- USB_LOGI(500, "msc_host_install..");
- const esp_err_t err = msc_host_install(&msc_config);
- if (err != ESP_OK) {
- USB_LOGE(5000, "msc_host_install failed %d", err);
- }
- JADE_ASSERT(err == ESP_OK);
+ const msc_host_driver_config_t config = {
+ .callback = msc_event_cb,
+ .create_backround_task = true,
+ .stack_size = 4096,
+ .task_priority = JADE_TASK_PRIO_USB,
+ };
+ err = msc_host_install(&config);
+ }
+ JADE_LOGD("msc_host_install returned %d", err);
+ usbstorage_state_set(err == ESP_OK ? USBSTATE_MSC_INSTALLED : USBSTATE_ERROR);
+ if (err != ESP_OK) {
+ goto cleanup;
}
- bool done = false;
-
- /* signal to usbstorage_start that we completed the start without [major] fail */
- xSemaphoreGive(main_task_semaphore);
-
- bool requires_host_uninstall = true;
- while (!done) {
- const TickType_t xTicksToWait = 100 / portTICK_PERIOD_MS;
-
- const EventBits_t event
- = xEventGroupWaitBits(usb_flags, DEVICE_CONNECTED | DEVICE_ADDRESS_MASK, pdTRUE, pdFALSE, xTicksToWait);
-
- if (!usbstorage_is_enabled) {
- break;
- }
-
- if (!(event & (DEVICE_CONNECTED | DEVICE_ADDRESS_MASK))) {
+ // Main loop. Handle msc events, mount storage once available
+ TickType_t wait_ticks = 50 / portTICK_PERIOD_MS;
+ while (true) {
+ msc_host_event_t ev;
+ if (xQueueReceive(usbstorage_msc_queue, &ev, wait_ticks) != pdPASS) {
+ if (usbstorage_state_get() & USBSTATE_SHUTDOWN_REQUESTED) {
+ JADE_LOGI("usb shutdown requested");
+ break;
+ }
continue;
}
-
- const uint8_t device_address = (event & DEVICE_ADDRESS_MASK) >> 4;
- trigger_event(USBSTORAGE_EVENT_DETECTED, device_address);
- for (;;) {
- const EventBits_t ebt = xEventGroupWaitBits(usb_flags, 0xFF, pdTRUE, pdFALSE, xTicksToWait);
- if (ebt & HOST_ALL_FREE) {
- // user removed the device which wasn't mounted
- trigger_event(USBSTORAGE_EVENT_EJECTED, device_address);
- done = !usbstorage_is_enabled;
+ if (ev.event == MSC_DEVICE_DISCONNECTED) {
+ JADE_LOGI("DEVICE_DISCONNECTED");
+ // Set the error condition even though this may not be an error
+ // (e.g. if the caller has finished their processing).
+ // If finished, the caller will not be checking the error state.
+ usbstorage_state_set(USBSTATE_ERROR);
+ break;
+ } else if (ev.event == MSC_DEVICE_CONNECTED) {
+ JADE_LOGI("msc_host_install_device %d..", (int)ev.device.address);
+ err = msc_host_install_device(ev.device.address, &msc_device);
+ JADE_LOGD("msc_host_install_device returned %d", err);
+ if (err != ESP_OK) {
+ usbstorage_state_set(USBSTATE_ERROR);
break;
- } else if (ebt & DEVICE_DISCONNECTED) {
- // user removed the device which was mounted!
- trigger_event(USBSTORAGE_EVENT_ABNORMALLY_EJECTED, device_address);
- done = !usbstorage_is_enabled;
+ }
+ JADE_LOGI("msc_host_vfs_register..");
+ const esp_vfs_fat_mount_config_t config = { .format_if_mount_failed = false, .max_files = 1 };
+ err = msc_host_vfs_register(msc_device, USBSTORAGE_MOUNT_POINT, &config, &vfs_handle);
+ JADE_LOGD("msc_host_vfs_register returned %d", err);
+ if (err != ESP_OK) {
+ usbstorage_state_set(USBSTATE_ERROR);
break;
- } else if (!ebt && requires_host_uninstall && !usb_device_installed) {
-
- // Stop powering any connected usb device to trigger detach events
- disable_usb_host();
-
- USB_LOGI(500, "msc_host_uninstall.. (1)");
- esp_err_t err = msc_host_uninstall();
- if (err == ESP_OK) {
- requires_host_uninstall = false;
- } else {
- USB_LOGE(5000, "msc_host_uninstall failed %d", err);
- }
}
+ // Let the caller know that usbstorage is available, and
+ // they can begin their processing.
+ JADE_LOGI("notify caller task");
+ xEventGroupSetBits(usbstorage_flags, USBSTORAGE_AVAILABLE);
+ // Wait longer for events so the callers task has more time to run
+ wait_ticks = 200 / portTICK_PERIOD_MS;
}
}
- usbstorage_register_callback(NULL, NULL);
-
- // This may fail if the user removes the device at the right time
- if (requires_host_uninstall) {
- USB_LOGI(500, "msc_host_uninstall.. (2)");
- const esp_err_t err = msc_host_uninstall();
- if (err != ESP_OK) {
- USB_LOGE(5000, "msc_host_uninstall failed %d", err);
- }
+
+cleanup:
+ const usbstorage_state_t state = usbstorage_state_get();
+ if (state & USBSTATE_ERROR) {
+ // Let the caller know an error occurred
+ JADE_LOGI("post error..");
+ xEventGroupSetBits(usbstorage_flags, USBSTORAGE_ERROR);
}
- usbstorage_is_enabled_subtask = false;
- xSemaphoreTake(aux_task_semaphore, portMAX_DELAY);
- vTaskDelete(aux_task);
- vEventGroupDelete(usb_flags);
+ if (vfs_handle) {
+ JADE_LOGI("msc_host_vfs_unregister..");
+ err = msc_host_vfs_unregister(vfs_handle);
+ JADE_LOGD("msc_host_vfs_unregister returned %d", err);
+ vfs_handle = NULL;
+ }
- USB_LOGI(500, "usb_host_uninstall..");
- const esp_err_t err = usb_host_uninstall();
- if (err != ESP_OK) {
- USB_LOGE(5000, "usb_host_uninstall failed %d", err);
+ if (msc_device) {
+ JADE_LOGI("msc_host_uninstall_device..");
+ err = msc_host_uninstall_device(msc_device);
+ JADE_LOGD("msc_host_uninstall_device returned %d", err);
+ msc_device = NULL;
}
- xSemaphoreGive(main_task_semaphore);
+ if (state & USBSTATE_MSC_INSTALLED) {
+ JADE_LOGI("msc_host_uninstall..");
+ err = msc_host_uninstall();
+ JADE_SEMAPHORE_TAKE(usbstorage_mutex);
+ usbstorage_state &= ~USBSTATE_MSC_INSTALLED;
+ JADE_SEMAPHORE_GIVE(usbstorage_mutex);
+ JADE_LOGD("msc_host_uninstall returned %d", err);
+ }
- // wait to be killed
- for (;;) {
- vTaskDelay(portMAX_DELAY);
+ if (worker_task) {
+ JADE_LOGD("waiting for usb worker shutdown..");
+ while (!(usbstorage_state_get() & USBSTATE_WORKER_SHUTDOWN)) {
+ vTaskDelay(100 / portTICK_PERIOD_MS);
+ }
}
+
+ JADE_LOGI("usb_host_uninstall..");
+ err = usb_host_uninstall();
+ JADE_LOGD("usb_host_uninstall returned %d", err);
+
+ JADE_LOGI("disable_usb_host");
+ disable_usb_host(); // Stop powering any connected usb device
+
+ JADE_SEMAPHORE_TAKE(usbstorage_mutex);
+ // Setting usbstorage_task to NULL lets usbstorage_stop() know we are stopped
+ usbstorage_task = NULL;
+ JADE_SEMAPHORE_GIVE(usbstorage_mutex);
+
+ vTaskDelete(NULL);
}
void usbstorage_init(void)
{
- JADE_ASSERT(!main_task_semaphore);
- JADE_ASSERT(!aux_task_semaphore);
- JADE_ASSERT(!interface_mutex);
- JADE_ASSERT(!callback_mutex);
- main_task_semaphore = xSemaphoreCreateBinary();
- aux_task_semaphore = xSemaphoreCreateBinary();
- interface_mutex = xSemaphoreCreateMutex();
- callback_mutex = xSemaphoreCreateMutex();
- JADE_ASSERT(main_task_semaphore);
- JADE_ASSERT(aux_task_semaphore);
- JADE_ASSERT(interface_mutex);
- JADE_ASSERT(callback_mutex);
+ JADE_ASSERT(!usbstorage_mutex && !usbstorage_flags && !usbstorage_msc_queue);
+ usbstorage_mutex = xSemaphoreCreateMutex();
+ usbstorage_flags = xEventGroupCreate();
+ usbstorage_msc_queue = xQueueCreate(3, sizeof(msc_host_event_t));
+ JADE_ASSERT(usbstorage_mutex && usbstorage_flags && usbstorage_msc_queue);
}
-bool usbstorage_start(void)
+EventGroupHandle_t usbstorage_start(void)
{
- JADE_ASSERT(main_task_semaphore);
- JADE_ASSERT(aux_task_semaphore);
- JADE_ASSERT(interface_mutex);
- JADE_SEMAPHORE_TAKE(interface_mutex);
- JADE_ASSERT(!usbstorage_is_enabled);
- JADE_ASSERT(!main_task);
-
- // Power any connected device
- JADE_ASSERT(!usb_is_powered());
- enable_usb_host();
+ JADE_LOGI("usbstorage_start");
+ JADE_ASSERT(usbstorage_mutex && usbstorage_flags && usbstorage_msc_queue);
- usbstorage_is_enabled = true;
- const BaseType_t task_created = xTaskCreatePinnedToCore(
- usbstorage_task, "usb_storage", 2 * 1024, NULL, JADE_TASK_PRIO_USB, &main_task, JADE_CORE_SECONDARY);
- JADE_ASSERT(task_created == pdPASS);
- JADE_ASSERT(main_task);
- xSemaphoreTake(main_task_semaphore, portMAX_DELAY);
- const bool enabled = usbstorage_is_enabled;
- JADE_SEMAPHORE_GIVE(interface_mutex);
- return enabled;
-}
+ JADE_SEMAPHORE_TAKE(usbstorage_mutex);
+ JADE_ASSERT(!usbstorage_task);
+ usbstorage_state = USBSTATE_NONE;
+ xEventGroupClearBits(usbstorage_flags, USBSTORAGE_AVAILABLE | USBSTORAGE_ERROR);
-void usbstorage_stop(void)
-{
- JADE_ASSERT(main_task);
- JADE_SEMAPHORE_TAKE(interface_mutex);
- JADE_ASSERT(usbstorage_is_enabled);
- usbstorage_is_enabled = false;
- xSemaphoreTake(main_task_semaphore, portMAX_DELAY);
+ // We must not be connected to power, i.e. USB cable
+ JADE_ASSERT(!usb_is_powered());
- vTaskDelete(main_task);
- main_task = NULL;
+#ifdef USBSTORAGE_DISABLE_LOGGING
+ // Record initial log level and set logging to NONE
+ initial_log_level = esp_log_level_get(NULL);
+ esp_log_level_set("*", ESP_LOG_NONE);
+#endif
- // Stop powering any connected usb device
- disable_usb_host();
+ // Start up the task that brings usb storage online
+ const BaseType_t retval = xTaskCreatePinnedToCore(
+ usbstorage_impl, "usb_storage", 4 * 1024, NULL, JADE_TASK_PRIO_USB, &usbstorage_task, JADE_CORE_SECONDARY);
+ JADE_SEMAPHORE_GIVE(usbstorage_mutex);
+ JADE_ASSERT(retval == pdPASS);
- JADE_SEMAPHORE_GIVE(interface_mutex);
+ // Wait until the task has started, failed or been shutdown
+ EventGroupHandle_t caller_events = NULL;
+ while (true) {
+ if (xSemaphoreTake(usbstorage_mutex, 10 / portTICK_PERIOD_MS) == pdTRUE) {
+ if (usbstorage_state & USBSTATE_TERMINAL_EVENT) {
+ break; // Failed or shutdown
+ } else if (usbstorage_state & USBSTATE_USB_INSTALLED) {
+ caller_events = usbstorage_flags;
+ break; // Startup is underway
+ }
+ xSemaphoreGive(usbstorage_mutex);
+ }
+ vTaskDelay(100 / portTICK_PERIOD_MS);
+ }
+ xSemaphoreGive(usbstorage_mutex);
+ return caller_events;
}
-bool usbstorage_mount(uint8_t device_address)
+void usbstorage_stop(void)
{
- JADE_SEMAPHORE_TAKE(interface_mutex);
- /* if any of these fails usually is because the device was removed */
- JADE_RETURN_CHECK(msc_host_install_device(device_address, &msc_device));
- usb_device_installed = true;
-
- JADE_RETURN_CHECK(msc_host_vfs_register(msc_device, USBSTORAGE_MOUNT_POINT, &mount_config, &vfs_handle));
- JADE_SEMAPHORE_GIVE(interface_mutex);
- return true;
-}
+ JADE_LOGI("usbstorage_stop");
+ JADE_ASSERT(usbstorage_mutex && usbstorage_flags && usbstorage_msc_queue);
-void usbstorage_unmount(void)
-{
- JADE_SEMAPHORE_TAKE(interface_mutex);
- // FIXME: on failure just send a callback rather than ERROR_CHECK?
- if (vfs_handle) {
- JADE_ERROR_CHECK(msc_host_vfs_unregister(vfs_handle));
- vfs_handle = NULL;
- }
- if (msc_device) {
- JADE_ERROR_CHECK(msc_host_uninstall_device(msc_device));
- msc_device = NULL;
+ // Signal usbstorage_task to shutdown/wait for it to do so
+ while (true) {
+ if (xSemaphoreTake(usbstorage_mutex, 10 / portTICK_PERIOD_MS) == pdTRUE) {
+ if (!usbstorage_task) {
+ xSemaphoreGive(usbstorage_mutex);
+ break; // usbstorage_task is shutdown
+ }
+ usbstorage_state |= USBSTATE_SHUTDOWN_REQUESTED;
+ xSemaphoreGive(usbstorage_mutex);
+ }
+ vTaskDelay(100 / portTICK_PERIOD_MS);
}
- usb_device_installed = false;
- JADE_SEMAPHORE_GIVE(interface_mutex);
+
+#ifdef USBSTORAGE_DISABLE_LOGGING
+ // Return to initial log level
+ esp_log_level_set("*", initial_log_level);
+ esp_log_level_set("nvs", ESP_LOG_ERROR); // As per storage_init()
+#endif
}
#endif // AMALGAMATED_BUILD
diff --git a/main/usbhmsc/usbhmsc.h b/main/usbhmsc/usbhmsc.h
index 8f89285..2b6ce0b 100644
--- a/main/usbhmsc/usbhmsc.h
+++ b/main/usbhmsc/usbhmsc.h
@@ -3,61 +3,24 @@
#include <freertos/FreeRTOS.h>
#include <freertos/task.h>
-#include <stdbool.h>
#define USBSTORAGE_MOUNT_POINT "/usb"
-#define USB_VISUAL_LOG false
-#define USB_VISUAL_LOG_LEVEL ESP_LOG_INFO
-#define USB_MESSAGE_ACTIVITY(delay, msg) \
- do { \
- if (USB_VISUAL_LOG) { \
- const char* message[] = { msg }; \
- display_message_activity(message, 1); \
- vTaskDelay(delay / portTICK_PERIOD_MS); \
- } \
- } while (false)
-#define USB_LOGE(delay, fmt, ...) \
- do { \
- JADE_LOGE(fmt, ##__VA_ARGS__); \
- if (USB_VISUAL_LOG && USB_VISUAL_LOG_LEVEL >= ESP_LOG_ERROR) { \
- char msg[128]; \
- snprintf(msg, sizeof(msg), fmt, ##__VA_ARGS__); \
- USB_MESSAGE_ACTIVITY(delay, msg); \
- } \
- } while (false)
-#define USB_LOGI(delay, fmt, ...) \
- do { \
- JADE_LOGI(fmt, ##__VA_ARGS__); \
- if (USB_VISUAL_LOG && USB_VISUAL_LOG_LEVEL >= ESP_LOG_INFO) { \
- char msg[128]; \
- snprintf(msg, sizeof(msg), fmt, ##__VA_ARGS__); \
- USB_MESSAGE_ACTIVITY(delay, msg); \
- } \
- } while (false)
-
typedef enum {
- USBSTORAGE_EVENT_DETECTED,
- USBSTORAGE_EVENT_EJECTED,
- USBSTORAGE_EVENT_ABNORMALLY_EJECTED,
+ USBSTORAGE_AVAILABLE = 0x1,
+ USBSTORAGE_ERROR = 0x2,
} usbstorage_event_t;
-typedef void (*usbstorage_callback_t)(usbstorage_event_t event, uint8_t device_address, void* ctx);
-
-/* this is required before usbstorage_start is called */
-void usbstorage_register_callback(usbstorage_callback_t callback, void* ctx);
-
/* this is called only once in main */
void usbstorage_init(void);
-/* call this any time you want to detect usb storage */
-bool usbstorage_start(void);
+/* Activate usb storage. If the return value is non-null, the caller will
+ * be signalled when usb storage is available at USBSTORAGE_MOUNT_POINT,
+ * or if an error occurs.
+ */
+EventGroupHandle_t usbstorage_start(void);
-/* this blocks until the drivers are uninstalled and tasks stopped/deleted */
+/* Shutdown usb storage. Blocks until the shutdown is complete */
void usbstorage_stop(void);
-bool usbstorage_mount(uint8_t device_address);
-
-void usbstorage_unmount(void);
-
#endif /* USBHMSC_H_ */
diff --git a/main/usbhmsc/usbmode.c b/main/usbhmsc/usbmode.c
index d146bbd..c74f50b 100644
--- a/main/usbhmsc/usbmode.c
+++ b/main/usbhmsc/usbmode.c
@@ -52,14 +52,6 @@ static const char SIGNED_PSBT_SUFFIX[] = "_signed.psbt";
// Function predicate to filter filenames available for a particular action
typedef bool (*filename_filter_fn_t)(const char* path, const char* filename, const size_t filename_len);
-// State of usb storage
-typedef enum { USBSTORAGE_STATE_NONE, USBSTORAGE_STATE_ERROR, USBSTORAGE_STATE_MOUNTED } usbstorage_state_t;
-
-struct usbstorage_ctx {
- SemaphoreHandle_t mutex;
- volatile usbstorage_state_t state;
-};
-
// Context object passed through to action callbacks
typedef struct {
const char* extra_path;
@@ -272,32 +264,6 @@ static bool select_file_from_filtered_list(const char* title, const char* const
return done;
}
-// usb storage state event callback
-static void handle_usbstorage_event(const usbstorage_event_t event, const uint8_t device_address, void* ctx)
-{
- struct usbstorage_ctx* const storage_ctx = (struct usbstorage_ctx*)ctx;
- JADE_ASSERT(storage_ctx && storage_ctx->mutex);
-
- usbstorage_state_t state = USBSTORAGE_STATE_NONE;
- if (event == USBSTORAGE_EVENT_DETECTED) {
- // Device detected: mount it immediately
- if (usbstorage_mount(device_address)) {
- state = USBSTORAGE_STATE_MOUNTED;
- } else {
- state = USBSTORAGE_STATE_ERROR;
- }
- } else {
- JADE_ASSERT(event == USBSTORAGE_EVENT_EJECTED || event == USBSTORAGE_EVENT_ABNORMALLY_EJECTED);
- state = USBSTORAGE_STATE_NONE; // Reset state when ejected
- }
- xSemaphoreTake(storage_ctx->mutex, portMAX_DELAY);
- storage_ctx->state = state;
- xSemaphoreGive(storage_ctx->mutex);
- if (state == USBSTORAGE_STATE_ERROR) {
- JADE_LOGE("Failed to mount USB storage!");
- }
-}
-
// Generic handler to run usb storage actions
static bool handle_usbstorage_action(const char* title, usbstorage_action_fn_t usbstorage_action,
const usbstorage_action_context_t* ctx, const bool async_action)
@@ -317,12 +283,8 @@ static bool handle_usbstorage_action(const char* title, usbstorage_action_fn_t u
display_processing_message_activity();
serial_stop();
- struct usbstorage_ctx storage_ctx = { xSemaphoreCreateMutex(), USBSTORAGE_STATE_NONE };
- JADE_ASSERT(storage_ctx.mutex);
- usbstorage_state_t state = USBSTORAGE_STATE_NONE;
- usbstorage_register_callback(handle_usbstorage_event, &storage_ctx);
-
- if (!usbstorage_start()) {
+ EventGroupHandle_t usbstorage_handle = usbstorage_start();
+ if (!usbstorage_handle) {
JADE_LOGE("Failed to start USB storage!");
const char* message[] = { "Failed to start", "usb storage!" };
await_error_activity(message, 2);
@@ -337,25 +299,26 @@ static bool handle_usbstorage_action(const char* title, usbstorage_action_fn_t u
gui_activity_t* act_prompt = NULL;
int counter = 0;
bool action_initiated = false;
+ EventBits_t usbstorage_events;
while (true) {
// Fetch the current state set by handle_usbstorage_event()
- xSemaphoreTake(storage_ctx.mutex, portMAX_DELAY);
- state = storage_ctx.state;
- xSemaphoreGive(storage_ctx.mutex);
+ usbstorage_events = xEventGroupWaitBits(
+ usbstorage_handle, USBSTORAGE_AVAILABLE | USBSTORAGE_ERROR, pdFALSE, pdFALSE, 100 / portTICK_PERIOD_MS);
- if (state == USBSTORAGE_STATE_MOUNTED) {
+ if (usbstorage_events & USBSTORAGE_ERROR) {
+ // Error accessing USB storage: Show error and exit
+ const char* message[] = { "Error accessing usb", "storage. Note: only", "FAT32 is supported." };
+ await_error_activity(message, 3);
+ break;
+ } else if (usbstorage_events == USBSTORAGE_AVAILABLE) {
// USB storage is mounted: run the action
if (act_prompt) {
gui_set_current_activity(prior_activity);
}
+ JADE_LOGI("starting usbstorage_action");
action_initiated = usbstorage_action(ctx);
break;
- } else if (state == USBSTORAGE_STATE_ERROR) {
- // Error accessing USB storage: Show error and exit
- const char* message[] = { "Error accessing usb", "storage. Note: only", "FAT32 is supported." };
- await_error_activity(message, 3);
- break;
}
// At this point, USB storage is not yet mounted
@@ -378,7 +341,6 @@ static bool handle_usbstorage_action(const char* title, usbstorage_action_fn_t u
act_prompt, GUI_BUTTON_EVENT, ESP_EVENT_ANY_ID, NULL, &ev_id, NULL, 100 / portTICK_PERIOD_MS)) {
if (ev_id == BTN_SETTINGS_USBSTORAGE_BACK) {
- usbstorage_register_callback(NULL, NULL);
// when the user goes back we go through here
// then the device hasn't started any action, but has disk detected
break;
@@ -394,23 +356,18 @@ static bool handle_usbstorage_action(const char* title, usbstorage_action_fn_t u
// If the action was not an async action (ie. it has already completed) or
// the action was never properly started, we stop/unmount usbstorage now.
if (!async_action || !action_initiated) {
- if (state != USBSTORAGE_STATE_NONE) {
- // if usb was detected it may need unmounting/uninstalling
- usbstorage_unmount();
- }
+ JADE_LOGI("stopping usb");
usbstorage_stop();
serial_start();
}
- usbstorage_register_callback(NULL, NULL);
- vSemaphoreDelete(storage_ctx.mutex);
return action_initiated;
}
// OTA
static void prepare_common_msg(CborEncoder* root_map_encoder, CborEncoder* root_encoder, const jade_msg_source_t source,
- const char* method, uint8_t* buffer, const size_t buffer_len, const bool has_params)
+ const char* method, uint8_t* buffer, const size_t buffer_len, const bool has_params, const int msg_id)
{
JADE_ASSERT(root_map_encoder);
JADE_ASSERT(root_encoder);
@@ -421,7 +378,13 @@ static void prepare_common_msg(CborEncoder* root_map_encoder, CborEncoder* root_
cbor_encoder_init(root_encoder, buffer, buffer_len, 0);
const CborError cberr = cbor_encoder_create_map(root_encoder, root_map_encoder, has_params ? 3 : 2);
JADE_ASSERT(cberr == CborNoError);
- add_string_to_map(root_map_encoder, "id", "0");
+
+ {
+ char buf[8];
+ int rc = snprintf(buf, sizeof(buf), "%d", msg_id);
+ JADE_ASSERT(rc > 0 && rc < sizeof(buf));
+ add_string_to_map(root_map_encoder, "id", buf);
+ }
add_string_to_map(root_map_encoder, "method", method);
}
@@ -434,11 +397,10 @@ static bool post_ota_message(const jade_msg_source_t source, const size_t fwsize
CborEncoder root_encoder;
CborEncoder root_map_encoder;
- // FIXME: check max size required?
- uint8_t buf[512 + 128];
+ uint8_t buf[256]; // sufficient
uint8_t* cbor_buf = buf + 1;
const bool has_params = true;
- prepare_common_msg(&root_map_encoder, &root_encoder, source, "ota", cbor_buf, sizeof(buf) - 1, has_params);
+ prepare_common_msg(&root_map_encoder, &root_encoder, source, "ota", cbor_buf, sizeof(buf) - 1, has_params, 0);
CborError cberr = cbor_encode_text_stringz(&root_map_encoder, "params");
JADE_ASSERT(cberr == CborNoError);
@@ -458,22 +420,24 @@ static bool post_ota_message(const jade_msg_source_t source, const size_t fwsize
buf[0] = source;
const size_t cbor_len = cbor_encoder_get_buffer_size(&root_encoder, cbor_buf);
+ JADE_ASSERT(cbor_len + 1 <= sizeof(buf));
+
return jade_process_push_in_message(buf, cbor_len + 1);
}
-static bool post_ota_data_message(const jade_msg_source_t source, uint8_t* data, size_t data_len)
+static bool post_ota_data_message(const jade_msg_source_t source, uint8_t* data, size_t data_len, const int msg_id)
{
JADE_ASSERT(data);
JADE_ASSERT(data_len);
- // FIXME: check max size required?
- uint8_t buf[JADE_OTA_BUF_SIZE + 128];
+ uint8_t buf[JADE_OTA_BUF_SIZE + 128]; // sufficient
uint8_t* cbor_buf = buf + 1;
CborEncoder root_encoder;
CborEncoder root_map_encoder;
const bool has_params = true;
- prepare_common_msg(&root_map_encoder, &root_encoder, source, "ota_data", cbor_buf, sizeof(buf) - 1, has_params);
+ prepare_common_msg(
+ &root_map_encoder, &root_encoder, source, "ota_data", cbor_buf, sizeof(buf) - 1, has_params, msg_id);
add_bytes_to_map(&root_map_encoder, "params", data, data_len);
const CborError cberr = cbor_encoder_close_container(&root_encoder, &root_map_encoder);
@@ -481,23 +445,25 @@ static bool post_ota_data_message(const jade_msg_source_t source, uint8_t* data,
buf[0] = source;
const size_t cbor_len = cbor_encoder_get_buffer_size(&root_encoder, cbor_buf);
+ JADE_ASSERT(cbor_len + 1 <= sizeof(buf));
return jade_process_push_in_message(buf, cbor_len + 1);
}
static bool post_ota_complete_message(const jade_msg_source_t source)
{
- // FIXME: check max size required?
- uint8_t buf[64];
+ uint8_t buf[64]; // sufficient
uint8_t* cbor_buf = buf + 1;
CborEncoder root_encoder;
CborEncoder root_map_encoder; // id, method
const bool has_params = false;
- prepare_common_msg(&root_map_encoder, &root_encoder, source, "ota_complete", cbor_buf, sizeof(buf) - 1, has_params);
+ prepare_common_msg(
+ &root_map_encoder, &root_encoder, source, "ota_complete", cbor_buf, sizeof(buf) - 1, has_params, 0);
const CborError cberr = cbor_encoder_close_container(&root_encoder, &root_map_encoder);
JADE_ASSERT(cberr == CborNoError);
buf[0] = source;
const size_t cbor_len = cbor_encoder_get_buffer_size(&root_encoder, cbor_buf);
+ JADE_ASSERT(cbor_len + 1 <= sizeof(buf));
return jade_process_push_in_message(buf, cbor_len + 1);
}
@@ -585,24 +551,31 @@ static bool handle_ota_reply(const uint8_t* msg, const size_t len, void* ctx)
return true;
}
-static bool wait_for_ota_replies(size_t num_replies, bool* is_ok)
+static bool wait_for_ota_replies(size_t num_replies, const bool wait_forever, bool* is_ok)
{
JADE_ASSERT(is_ok);
+ bool any_failed = false;
*is_ok = false;
-
for (size_t i = 0; i < num_replies; ++i) {
// Wait for a reply message from the OTA process
int num_waits = 0;
while (!jade_process_get_out_message(handle_ota_reply, SOURCE_INTERNAL, is_ok)) {
- // No reply yet: Keep waiting up to ~5s for our message to be processed
- ++num_waits;
- if (++num_waits > 100) {
+ // No reply yet: Keep waiting for our message to be processed.
+ // Wait forever while waiting for user confirmation, and 10
+ // seconds once data has begun being transferred.
+ // Note jade_process_get_out_message() waits up to 20ms for messages.
+ const int max_waits = 10000 / 20;
+ if (!wait_forever && ++num_waits > max_waits) {
+ JADE_LOGE("wait_for_ota_replies timeout");
return false; // Timed out waiting
}
- // jade_process_get_out_message() already waited 20ms, wait another 30
- vTaskDelay(30 / portTICK_PERIOD_MS);
}
+ any_failed |= !*is_ok;
}
+ if (any_failed) {
+ *is_ok = false;
+ }
+ JADE_LOGD("wait_for_ota_replies: %d replies, OK=%d", (int)num_replies, *is_ok ? 1 : 0);
return true; // Successfully waited for all messages
}
@@ -617,8 +590,8 @@ static void usbmode_ota_worker(void* ctx)
JADE_ASSERT(ctx_data && ctx_data->file_to_flash && ctx_data->data_to_send != 0);
uint8_t buffer[JADE_OTA_BUF_SIZE];
- size_t msgs_sent = 1; // Initially just an "ota" message sent
- size_t msgs_waited = 0;
+ int msgs_sent = 1; // Initially just an "ota" message sent
+ int msgs_waited = 0;
const int fd = open(ctx_data->file_to_flash, O_RDONLY, 0);
free(ctx_data->file_to_flash);
ctx_data->file_to_flash = NULL;
@@ -626,11 +599,12 @@ static void usbmode_ota_worker(void* ctx)
// Loop passing our ota data to the ota task
bool failed_wait = false;
while (ctx_data->data_to_send) {
- if (msgs_sent > 2) {
+ if (msgs_sent > 1) {
// Wait for the n-1th message that we sent. This allows this task to stay
// ahead of the ota task so that both can work in parallel.
+ const bool wait_forever = msgs_sent <= 4;
bool ok = false;
- failed_wait = !wait_for_ota_replies(1, &ok);
+ failed_wait = !wait_for_ota_replies(1, wait_forever, &ok);
if (failed_wait) {
// Failed to get a reply: The ota task is dead/not responding
break;
@@ -650,8 +624,9 @@ static void usbmode_ota_worker(void* ctx)
// e.g. the device was unplugged or is unreliable.
break;
}
- const bool res = post_ota_data_message(SOURCE_INTERNAL, buffer, bytes_read);
+ const bool res = post_ota_data_message(SOURCE_INTERNAL, buffer, bytes_read, msgs_sent);
JADE_ASSERT(res);
+ JADE_LOGD("posted ota_data message %d", msgs_sent);
++msgs_sent;
ctx_data->data_to_send -= bytes_read;
}
@@ -671,7 +646,8 @@ static void usbmode_ota_worker(void* ctx)
++msgs_sent;
bool ok = false;
// Wait for any outstanding ota replies
- failed_wait = !wait_for_ota_replies(msgs_sent - msgs_waited, &ok);
+ const bool wait_forever = false;
+ failed_wait = !wait_for_ota_replies(msgs_sent - msgs_waited, wait_forever, &ok);
}
// If the ota succeeded the device will be rebooted soon.
@@ -681,7 +657,7 @@ static void usbmode_ota_worker(void* ctx)
// TODO: Notify the user in the failed_wait == true case.
// After ota try to unmount usbstorage and restart normal serial comms
- usbstorage_unmount();
+ JADE_LOGI("OTA complete: stopping usb");
usbstorage_stop();
serial_start();
vTaskDelete(NULL);
@@ -744,15 +720,17 @@ static bool initiate_usb_ota(const usbstorage_action_context_t* ctx)
return false;
}
- char hash_filename[MAX_FILENAME_SIZE];
- const int ret = snprintf(hash_filename, sizeof(hash_filename), "%s%s", filename, HASH_SUFFIX);
- JADE_ASSERT(ret > 0 && ret < sizeof(hash_filename));
-
uint8_t hash[SHA256_LEN];
- if (!read_hash_file_to_buffer(hash_filename, hash, sizeof(hash))) {
- const char* message[] = { "Failed to read", "hash file" };
- await_error_activity(message, 2);
- return false;
+ {
+ char hash_filename[MAX_FILENAME_SIZE];
+ const int ret = snprintf(hash_filename, sizeof(hash_filename), "%s%s", filename, HASH_SUFFIX);
+ JADE_ASSERT(ret > 0 && ret < sizeof(hash_filename));
+
+ if (!read_hash_file_to_buffer(hash_filename, hash, sizeof(hash))) {
+ const char* message[] = { "Failed to read", "hash file" };
+ await_error_activity(message, 2);
+ return false;
+ }
}
const size_t cmpsize = get_file_size(filename);
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.