feat(core): add more robust USB configuration status for battery powered devices
What the developer wrote
Authored by tychovrahe
feat(core): add more robust USB configuration status for battery powered devices
[no changelog]
What changed, and why it matters
This commit changes how a Trezor hardware wallet decides whether its USB connection is truly active, especially for battery-powered models. Previously, the code assumed the device was USB-powered and treated a suspended USB state as still 'configured' and ready. The patch makes that behavior depend on actual power-manager data and a remembered 'was configured' flag, so the device no longer falsely reports itself as ready when it may not be. This is a hardening/robustness improvement rather than a clear-cut fix for an exploitable vulnerability.
Treat as a defensive hardening commit. Review whether the prior behavior could have led to premature USB communication readiness on battery-powered devices, and verify that the new `pm_get_state()` path correctly handles all suspend/resume and disconnect scenarios. No immediate incident response is indicated from the diff alone.
Security signals we found
State-machine robustness improvement for USB configured/suspended detection
Removal of unconditional `powered_from_usb = sectrue` assumption when power manager is present
Addition of explicit power-manager connectivity check before reporting suspended device as ready
Persistent 'was_configured' flag to avoid relying solely on STM32 USB old-state field
Glitch-tolerance logic now restricted to USB-powered devices
Evidence from the diff
The change is in core/embed/io/usb/stm32/usb.c. It introduces drv->was_configured to track whether the USB device ever reached USBD_STATE_CONFIGURED, replacing a check against pdev->dev_old_state. When USE_POWER_MANAGER is defined, it now queries pm_get_state() and only treats a suspended device as ready if state.usb_connected is true. The 2-second ‘ready recently’ glitch workaround is now gated on powered_from_usb == sectrue. The patch reduces false-positive ‘configured’ reporting on battery-powered devices and removes the unconditional assumption that the device is USB-powered.
Changed components
core/embed/io/usb/stm32/usb.cUSB driver state machinePower manager integration (USE_POWER_MANAGER)Battery-powered Trezor modelsInspect captured patch +41 / −17
diff --git a/core/embed/io/usb/stm32/usb.c b/core/embed/io/usb/stm32/usb.c
index 8dae4108..de6648a3 100644
--- a/core/embed/io/usb/stm32/usb.c
+++ b/core/embed/io/usb/stm32/usb.c
@@ -28,6 +28,10 @@
#include <sys/sysevent_source.h>
#include <sys/systick.h>
+#ifdef USE_POWER_MANAGER
+#include <io/power_manager.h>
+#endif
+
#include "usb_internal.h"
#define USB_MAX_CONFIG_DESC_SIZE 256
@@ -89,6 +93,8 @@ typedef struct {
uint32_t ready_time;
// Set to `sectrue` if the USB stack was ready sinced the last start
secbool was_ready;
+ // Old state configured
+ bool was_configured;
// Task local storage for USB driver
usb_driver_tls_t tls[SYSTASK_MAX_TASKS];
@@ -274,22 +280,36 @@ static secbool usb_configured(void) {
return secfalse;
}
- secbool powered_from_usb = sectrue; // TODO
+#ifdef USE_POWER_MANAGER
+ secbool powered_from_usb = secfalse;
+#else
+ secbool powered_from_usb = sectrue;
+#endif
secbool ready = secfalse;
if (pdev->dev_state == USBD_STATE_CONFIGURED) {
// USB is configured, ready to transfer data
ready = sectrue;
- } else if (pdev->dev_state == USBD_STATE_SUSPENDED &&
- pdev->dev_old_state == USBD_STATE_CONFIGURED) {
+ drv->was_configured = true;
+ } else if (pdev->dev_state == USBD_STATE_SUSPENDED && drv->was_configured) {
// USB is suspended, but was configured before
//
// Linux has autosuspend device after 2 seconds by default.
// So a suspended device that was seen as configured is reported as
// configured.
- //
+
+#ifdef USE_POWER_MANAGER
+ pm_state_t state = {0};
+ pm_status_t status = pm_get_state(&state);
+
+ if (status == PM_OK && state.usb_connected) {
+ ready = sectrue;
+ }
+#else
ready = sectrue;
+#endif
+
} else if ((drv->was_ready == secfalse) && (powered_from_usb == sectrue)) {
// First run after the startup with USB power
drv->was_ready = sectrue;
@@ -298,24 +318,28 @@ static secbool usb_configured(void) {
uint32_t now = hal_ticks_ms();
- if (ready == sectrue) {
- irq_key_t irq_key = irq_lock();
- drv->ready_time = now;
- irq_unlock(irq_key);
- } else {
- // This is a workaround to handle the glitches in the USB connection,
- // especially for USB-powered-only devices. This should be
- // revisited and probably fixed elsewhere.
+ if (sectrue == powered_from_usb) {
+ if (ready == sectrue) {
+ irq_key_t irq_key = irq_lock();
+ drv->ready_time = now;
+ irq_unlock(irq_key);
+ } else {
+ // This is a workaround to handle the glitches in the USB connection,
+ // especially for USB-powered-only devices. This should be
+ // revisited and probably fixed elsewhere.
- irq_key_t irq_key = irq_lock();
- bool ready_recently = (int32_t)(now - drv->ready_time) < 2000;
- irq_unlock(irq_key);
+ irq_key_t irq_key = irq_lock();
+ bool ready_recently = (int32_t)(now - drv->ready_time) < 2000;
+ irq_unlock(irq_key);
- if ((drv->was_ready == sectrue) && ready_recently) {
- ready = sectrue;
+ if ((drv->was_ready == sectrue) && ready_recently) {
+ ready = sectrue;
+ }
}
}
+ drv->was_configured = sectrue == ready;
+
return ready;
}
Why this scored 24/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.