What changed, and why it matters
This commit refactors how the Trezor device decides when to dim the screen, lock itself, and suspend when running on battery power. It centralizes those settings and changes the timing of suspend so it happens after active workflows finish rather than immediately. The changes appear to be a reliability/optimization fix rather than a clear security patch, but they touch the lock and suspend logic that protects the device when unattended.
Treat as a routine optimization/refactoring commit. If reviewing for security, verify that suspend-after-workflow does not create a window where an unlocked device remains accessible longer than intended, that IdleTimer.clear() does not cancel safety-critical callbacks, and that the removed emulator guard does not introduce emulator-only behavior differences.
Security signals we found
Touches device lock/suspend/autolock power-management paths
Changes timing of suspend from immediate to after-workflow in some cases
Adds IdleTimer.clear() and loop.clear() during bootscreen exit, which may affect cleanup of background tasks
Removes emulator-specific guard around suspend scheduling
Changes mailbox notification behavior to replace queued items
Evidence from the diff
The patch modifies lock_manager.py, boot.py, storage/device.py, and workflow.py. Key changes: (1) introduces configure_autodim() to centralize idle_timer registration of autodim_display and battery autolock; (2) moves AUTODIM_DELAY_MS into storage; (3) renames _prepare_suspend to _schedule_suspend_after_workflow and removes the emulator guard; (4) splits _suspend_and_resume into a synchronous suspend_and_resume() and an async wrapper _suspend_and_resume_task(); (5) changes lock_device_if_unlocked() to suspend immediately when autolock_interrupts_workflow is true, otherwise schedule suspend after workflow; (6) adds IdleTimer.clear() and loop.clear() calls in bootscreen(); (7) uses replace=True on mailbox put to avoid queue growth. The commit is titled ‘optimize lock_manager’ and contains no explicit security disclosure.
Changed components
core/src/apps/common/lock_manager.pycore/src/boot.pycore/src/storage/device.pycore/src/trezor/workflow.pyInspect captured patch +51 / −32
diff --git a/core/src/apps/common/lock_manager.py b/core/src/apps/common/lock_manager.py
index 9ec6dab21..f95d48b49 100644
--- a/core/src/apps/common/lock_manager.py
+++ b/core/src/apps/common/lock_manager.py
@@ -29,21 +29,21 @@ else:
_SHOULD_SUSPEND = False
_notify_power_button: loop.mailbox[None] = loop.mailbox()
- def _prepare_suspend() -> None:
- """Signal that the device should be suspended by the default task.
+ def _schedule_suspend_after_workflow() -> None:
+ """Signal that the device should be suspended by the default task after the
+ running workflows finish.
- Sets a suspend homescreen for next time the default task is invoked."""
- if not utils.EMULATOR:
- # FIXME: suspend not implemented on emulator
- _SHOULD_SUSPEND = True
- set_homescreen()
+ Sets a suspend homescreen for next time the default task is invoked.
+ """
+ _SHOULD_SUSPEND = True
+ set_homescreen()
def notify_suspend() -> None:
"""Signal that the the device should be suspended in the next cycle.
Notifies an asynchronous task to perform the suspend in a separate thread.
"""
- _notify_power_button.put(None)
+ _notify_power_button.put(None, replace=True)
async def _power_handler() -> None:
"""Handler for the notify_suspend signal."""
@@ -51,27 +51,43 @@ else:
await _notify_power_button
lock_device_if_unlocked()
- async def _suspend_and_resume() -> None:
- """Default task that suspends the device and invokes resumption.
+ def suspend_and_resume() -> None:
+ """Suspend Trezor and handle wakeup.
- Must be async (or more precisely a generator) so that we can schedule it
- via set_default."""
+ The function will only return after Trezor has woken up.
+ """
from trezor.ui import CURRENT_LAYOUT
wakeup_flag = suspend_device()
if wakeup_flag == io.pm.WAKEUP_FLAG_BUTTON:
+ workflow.idle_timer.touch()
if CURRENT_LAYOUT is not None:
CURRENT_LAYOUT.layout.request_complete_repaint()
_SHOULD_SUSPEND = False
set_homescreen()
+ async def _suspend_and_resume_task() -> None:
+ """Task to suspend Trezor and handle wakeup.
+
+ Must be async so that we can schedule it via set_default.
+ """
+ suspend_and_resume()
+
def lock_device_if_unlocked_on_battery() -> None:
"""Lock the device if it is unlocked and running on battery or wireless charger."""
if not io.pm.is_usb_connected():
lock_device_if_unlocked()
+ def configure_autodim() -> None:
+ """Configure the autodim setting via idle timer."""
+ workflow.idle_timer.set(storage_device.AUTODIM_DELAY_MS, autodim_display)
+ workflow.idle_timer.set(
+ storage_device.get_autolock_delay_battery_ms(),
+ lock_device_if_unlocked_on_battery,
+ )
+
def set_homescreen() -> None:
import storage.recovery as storage_recovery
@@ -81,7 +97,7 @@ def set_homescreen() -> None:
set_default = workflow.set_default # local_cache_attribute
if utils.USE_POWER_MANAGER and _SHOULD_SUSPEND:
- set_default(_suspend_and_resume)
+ set_default(_suspend_and_resume_task)
elif context.cache_is_set(APP_COMMON_BUSY_DEADLINE_MS):
from apps.homescreen import busyscreen
@@ -133,7 +149,12 @@ def lock_device_if_unlocked() -> None:
lock_device(interrupt_workflow=workflow.autolock_interrupts_workflow)
if utils.USE_POWER_MANAGER:
- _prepare_suspend()
+ if workflow.autolock_interrupts_workflow:
+ # suspend immediately
+ suspend_and_resume()
+ else:
+ # set a suspending homescreen
+ _schedule_suspend_after_workflow()
async def unlock_device() -> None:
@@ -175,12 +196,8 @@ def reload_settings_from_storage() -> None:
)
if utils.USE_POWER_MANAGER:
- # autodim setting is not from storage but keeping it here for simplicity
- workflow.idle_timer.set(30_000, autodim_display)
- workflow.idle_timer.set(
- storage_device.get_autolock_delay_battery_ms(),
- lock_device_if_unlocked_on_battery,
- )
+ configure_autodim()
+
wire.message_handler.EXPERIMENTAL_ENABLED = (
storage_device.get_experimental_features()
)
diff --git a/core/src/boot.py b/core/src/boot.py
index c458f66ba..59ae0d32a 100644
--- a/core/src/boot.py
+++ b/core/src/boot.py
@@ -25,8 +25,7 @@ if utils.USE_OPTIGA:
if utils.USE_POWER_MANAGER:
from trezor import workflow
- from trezor.power_management.autodim import autodim_display
- from apps.common.lock_manager import lock_device_if_unlocked_on_battery
+ from apps.common.lock_manager import configure_autodim, boot as boot_power_manager
# have to use "==" over "in (list)" so that it can be statically replaced
# with the correct value during the build process
@@ -61,12 +60,7 @@ async def bootscreen() -> None:
Allowing all of them before returning.
"""
if utils.USE_POWER_MANAGER:
- workflow.idle_timer.set(30_000, autodim_display)
- workflow.idle_timer.set(
- storage.device.get_autolock_delay_battery_ms(),
- lock_device_if_unlocked_on_battery,
- )
- workflow.autolock_interrupts_workflow = False
+ configure_autodim()
while True:
try:
@@ -120,10 +114,8 @@ async def bootscreen() -> None:
log.exception(__name__, e)
utils.halt(e.__class__.__name__)
- if utils.USE_POWER_MANAGER:
- workflow.idle_timer.remove(autodim_display)
- workflow.idle_timer.remove(lock_device_if_unlocked_on_battery)
- workflow.autolock_interrupts_workflow = True
+ workflow.idle_timer.clear()
+ loop.clear()
# Display emulator warning.
@@ -136,6 +128,8 @@ if not utils.USE_OPTIGA or (optiga.get_sec() or 0) < 150:
config.init(show_pin_timeout)
translations.init()
+if utils.USE_POWER_MANAGER:
+ boot_power_manager()
if __debug__ and not utils.EMULATOR:
config.wipe()
diff --git a/core/src/storage/device.py b/core/src/storage/device.py
index e3e2b4a0d..20caca945 100644
--- a/core/src/storage/device.py
+++ b/core/src/storage/device.py
@@ -67,6 +67,7 @@ AUTOLOCK_DELAY_USB_DEFAULT_MS = const(10 * 60 * 1000) # 10 minutes
AUTOLOCK_DELAY_USB_MAX_MS = const(0x2000_0000) # ~6 days
if utils.USE_POWER_MANAGER:
+ AUTODIM_DELAY_MS = 30 * 1000 # 30 seconds
AUTOLOCK_DELAY_BATT_MIN_MS = 30 * 1000 # 30 seconds
AUTOLOCK_DELAY_BATT_DEFAULT_MS = const(40 * 1000) # 40 seconds
AUTOLOCK_DELAY_BATT_MAX_MS = const(10 * 60 * 1000) # 10 minutes
diff --git a/core/src/trezor/workflow.py b/core/src/trezor/workflow.py
index 2570bc619..41b7d94be 100644
--- a/core/src/trezor/workflow.py
+++ b/core/src/trezor/workflow.py
@@ -269,6 +269,13 @@ class IdleTimer:
if task is not None:
loop.close(task)
+ def clear(self) -> None:
+ """Clear all idle callbacks."""
+ for _, task in self.tasks.items():
+ loop.close(task)
+ self.timeouts.clear()
+ self.tasks.clear()
+
idle_timer = IdleTimer()
"""Global idle timer."""
Why this scored 26/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.