refactor(core): dma2d_wait() busy wait removal
What changed, and why it matters
This commit refactors how the Trezor hardware wallet's graphics chip (DMA2D) waits for image-copying operations to finish. Previously the CPU burned power in a tight loop; now it puts the CPU to sleep and wakes it when the transfer completes. The change introduces new state tracking and error handling, but it also manipulates low-level CPU sleep/interrupt settings and manually clears locks and flags. There is no claim in the commit that this fixes a security bug, and no external advisory is provided. The main concern is whether the new wait logic could, in rare timing situations, leave the driver in an inconsistent state that affects screen rendering or stability, rather than a direct exploit path.
Treat as a non-security refactor unless additional context emerges. Review the WFE/SEVONPEND interaction for race conditions, verify that __HAL_UNLOCK cannot release a lock owned by another context, and ensure the timeout and abort path cannot deadlock or corrupt subsequent DMA2D operations. Regression testing should cover rapid init/deinit and concurrent rendering paths.
Security signals we found
Refactor of low-level DMA2D synchronization from busy-wait to WFE/SEVONPEND sleep-wake
Manual __HAL_UNLOCK and NVIC_ClearPendingIRQ added to recover HAL state
New dma_transfer_in_progress flag used to gate wait logic
Timeout-based fallback (10 ms) with abort-and-reset on error/timeout
No changelog entry and no security framing by vendor
Evidence from the diff
The patch replaces HAL_DMA2D_PollForTransfer() busy-waiting in dma2d_wait() with a WFE-based event loop using SEVONPEND, a 10 ms timeout, and manual flag/lock cleanup. It adds a volatile dma_transfer_in_progress flag set on successful HAL_DMA2D_Start/BlendingStart and cleared in dma2d_wait(). Error paths now call HAL_DMA2D_Abort(), re-enable DMA2D interrupts, clear flags/IRQ, and __HAL_UNLOCK the handle. The change touches only core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.c. No vendor security disclosure or researcher attribution is present in the commit or supplied references.
Changed components
core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.cTrezor Core firmware DMA2D graphics blit driverSTM32 DMA2D peripheral wait/synchronization pathInspect captured patch +253 / −60
diff --git a/core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.c b/core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.c
index db8f0323..2c675d96 100644
--- a/core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.c
+++ b/core/embed/io/gfx/bitblt/stm32/dma2d_bitblt.c
@@ -28,15 +28,24 @@
#include <io/dma2d_bitblt.h>
#include <io/gfx_color.h>
+#include <sys/irq.h>
+#include <sys/systick.h>
// Number of DMA2D layers - background (0) and foreground (1)
#define DMA2D_LAYER_COUNT 2
+// Timeout for waiting for DMA2D transfer completion in milliseconds.
+#define DMA2D_TIMEOUT_MS 10
+
typedef struct {
// Set if the driver is initialized
bool initialized;
// ST DMA2D driver handle
DMA2D_HandleTypeDef handle;
+
+ // Tracking of ongoing DMA transfer.
+ volatile bool dma_transfer_in_progress;
+
// CLUT cache
struct {
gfx_color32_t c_fg;
@@ -64,6 +73,17 @@ static inline bool dma2d_accessible(const void* ptr) {
#endif
}
+// DMA start failed: ensure no transfer is marked in progress and reset DMA2D
+// state.
+static inline void dma2d_error_handler(void) {
+ dma2d_driver_t* drv = &g_dma2d_driver;
+
+ drv->dma_transfer_in_progress = false;
+ HAL_DMA2D_Abort(&drv->handle);
+ // Re-enable interrupts to be prepared for next usage.
+ __HAL_DMA2D_ENABLE_IT(&drv->handle, DMA2D_IT_TC | DMA2D_IT_TE | DMA2D_IT_CE);
+}
+
void dma2d_init(void) {
dma2d_driver_t* drv = &g_dma2d_driver;
if (drv->initialized) {
@@ -76,12 +96,28 @@ void dma2d_init(void) {
__HAL_RCC_DMA2D_RELEASE_RESET();
__HAL_RCC_DMA2D_CLK_ENABLE();
+ // Disable NVIC DMA2D_IRQn (precaution).
+ NVIC_DisableIRQ(DMA2D_IRQn);
+
+ // Enable the transfer complete, transfer error and configuration error
+ // interrupts (used for waking up from sleep in dma2d_wait()).
+ __HAL_DMA2D_ENABLE_IT(&drv->handle, DMA2D_IT_TC | DMA2D_IT_TE | DMA2D_IT_CE);
+ drv->dma_transfer_in_progress = false;
+
drv->initialized = true;
}
void dma2d_deinit(void) {
dma2d_driver_t* drv = &g_dma2d_driver;
+ if (!drv->initialized) {
+ return;
+ }
+
+ HAL_DMA2D_Abort(&drv->handle);
+
+ __HAL_DMA2D_DISABLE_IT(&drv->handle, DMA2D_IT_TC | DMA2D_IT_TE | DMA2D_IT_CE);
+
__HAL_RCC_DMA2D_CLK_DISABLE();
__HAL_RCC_DMA2D_FORCE_RESET();
__HAL_RCC_DMA2D_RELEASE_RESET();
@@ -91,13 +127,74 @@ void dma2d_deinit(void) {
void dma2d_wait(void) {
dma2d_driver_t* drv = &g_dma2d_driver;
+ bool timeout_occurred = false;
if (!drv->initialized) {
return;
}
- while (HAL_DMA2D_PollForTransfer(&drv->handle, 10) != HAL_OK)
- ;
+ if (!drv->dma_transfer_in_progress) {
+ return;
+ }
+
+ if (!__HAL_DMA2D_GET_FLAG(&drv->handle,
+ DMA2D_FLAG_TC | DMA2D_FLAG_TE | DMA2D_FLAG_CE)) {
+ irq_key_t key = irq_lock();
+ // Enabled events and all interrupts, including disabled interrupts, can
+ // wakeup the processor put into sleep via WFE (Wait For Event) instruction.
+ uint32_t scb_scr_sevonpend_bkp = READ_BIT(SCB->SCR, SCB_SCR_SEVONPEND_Msk);
+ SET_BIT(SCB->SCR, SCB_SCR_SEVONPEND_Msk);
+ irq_unlock(key);
+
+ uint32_t timeout = ticks_timeout(DMA2D_TIMEOUT_MS);
+
+ // It is recommended to execute the SEV instruction (to generate the event)
+ // before falling asleep (the WFE instruction consumes it i.e. clears it and
+ // the next one will react on the event being expected).
+ __SEV();
+ __WFE();
+
+ // Periodically check the DMA2D transfer status until it is complete or
+ // an error occurs.
+ while (!__HAL_DMA2D_GET_FLAG(
+ &drv->handle, DMA2D_FLAG_TC | DMA2D_FLAG_TE | DMA2D_FLAG_CE)) {
+ // Ensure that all memory accesses are completed before checking the flag.
+ __DSB();
+ __WFE();
+
+ if (ticks_expired(timeout)) {
+ timeout_occurred = true;
+ break;
+ }
+ }
+
+ key = irq_lock();
+ // Restore SEVONPEND state
+ if (READ_BIT(scb_scr_sevonpend_bkp, SCB_SCR_SEVONPEND_Msk) == 0) {
+ CLEAR_BIT(SCB->SCR, SCB_SCR_SEVONPEND_Msk);
+ }
+ irq_unlock(key);
+ }
+
+ if (__HAL_DMA2D_GET_FLAG(&drv->handle, DMA2D_FLAG_TE | DMA2D_FLAG_CE) ||
+ timeout_occurred) {
+ HAL_DMA2D_Abort(&drv->handle);
+ // Re-enable interrupts to be prepared for next usage.
+ __HAL_DMA2D_ENABLE_IT(&drv->handle,
+ DMA2D_IT_TC | DMA2D_IT_TE | DMA2D_IT_CE);
+ }
+
+ // Clear all pending flags and pending IRQ to be prepared for next usage.
+ __HAL_DMA2D_CLEAR_FLAG(&drv->handle,
+ DMA2D_FLAG_TC | DMA2D_FLAG_TE | DMA2D_FLAG_CE);
+ __NVIC_ClearPendingIRQ(DMA2D_IRQn);
+
+ // Necessary to unlock HAL DMA2D handle to be prepared for next usage. It's
+ // usually done within the HAL_DMA2D_IRQHandler() called from the interrupt
+ // handler or HAL_DMA2D_PollForTransfer() or HAL_DMA2D_Abort() functions.
+ __HAL_UNLOCK(&drv->handle);
+
+ drv->dma_transfer_in_progress = false;
}
bool dma2d_rgb565_fill(const gfx_bitblt_t* bb) {
@@ -124,9 +221,15 @@ bool dma2d_rgb565_fill(const gfx_bitblt_t* bb) {
bb->dst_stride / sizeof(uint16_t) - bb->width;
HAL_DMA2D_Init(&drv->handle);
- HAL_DMA2D_Start(&drv->handle, gfx_color_to_color32(bb->src_fg),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, gfx_color_to_color32(bb->src_fg),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
} else {
#ifdef STM32U5
drv->handle.Init.ColorMode = DMA2D_OUTPUT_RGB565;
@@ -148,11 +251,17 @@ bool dma2d_rgb565_fill(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(
- &drv->handle, gfx_color_to_color32(bb->src_fg),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t), bb->width,
- bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, gfx_color_to_color32(bb->src_fg),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
#else
// STM32F4 can not accelerate blending with the fixed color
return false;
@@ -285,9 +394,16 @@ bool dma2d_rgb565_copy_mono4(const gfx_bitblt_t* params) {
dma2d_config_clut(1, gfx_color_to_color32(bb->src_fg),
gfx_color_to_color32(bb->src_bg));
- HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
return true;
}
@@ -320,10 +436,17 @@ bool dma2d_rgb565_copy_rgb565(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[1].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 1);
- HAL_DMA2D_Start(&drv->handle,
- (uint32_t)bb->src_row + bb->src_x * sizeof(uint16_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle,
+ (uint32_t)bb->src_row + bb->src_x * sizeof(uint16_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
return true;
}
@@ -420,11 +543,16 @@ bool dma2d_rgb565_blend_mono4(const gfx_bitblt_t* params) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(
- &drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t), bb->width,
- bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
}
return true;
@@ -465,10 +593,16 @@ bool dma2d_rgb565_blend_mono8(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(&drv->handle, (uint32_t)bb->src_row + bb->src_x,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
- bb->width, bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, (uint32_t)bb->src_row + bb->src_x,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint16_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
return true;
}
@@ -497,9 +631,16 @@ bool dma2d_rgba8888_fill(const gfx_bitblt_t* bb) {
bb->dst_stride / sizeof(uint32_t) - bb->width;
HAL_DMA2D_Init(&drv->handle);
- HAL_DMA2D_Start(&drv->handle, gfx_color_to_color32(bb->src_fg),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, gfx_color_to_color32(bb->src_fg),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
} else {
#ifdef STM32U5
drv->handle.Init.ColorMode = DMA2D_OUTPUT_ARGB8888;
@@ -521,11 +662,17 @@ bool dma2d_rgba8888_fill(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(
- &drv->handle, gfx_color_to_color32(bb->src_fg),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t), bb->width,
- bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, gfx_color_to_color32(bb->src_fg),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
#else
// STM32F4 can not accelerate blending with the fixed color
return false;
@@ -620,9 +767,16 @@ bool dma2d_rgba8888_copy_mono4(const gfx_bitblt_t* params) {
dma2d_config_clut(1, gfx_color_to_color32(bb->src_fg),
gfx_color_to_color32(bb->src_bg));
- HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
return true;
}
@@ -655,10 +809,17 @@ bool dma2d_rgba8888_copy_rgb565(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[1].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 1);
- HAL_DMA2D_Start(&drv->handle,
- (uint32_t)bb->src_row + bb->src_x * sizeof(uint16_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle,
+ (uint32_t)bb->src_row + bb->src_x * sizeof(uint16_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
return true;
}
@@ -755,11 +916,16 @@ bool dma2d_rgba8888_blend_mono4(const gfx_bitblt_t* params) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(
- &drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t), bb->width,
- bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, (uint32_t)bb->src_row + bb->src_x / 2,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
}
return true;
@@ -803,10 +969,16 @@ bool dma2d_rgba8888_blend_mono8(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[0].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 0);
- HAL_DMA2D_BlendingStart(&drv->handle, (uint32_t)bb->src_row + bb->src_x,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK == HAL_DMA2D_BlendingStart(
+ &drv->handle, (uint32_t)bb->src_row + bb->src_x,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
return true;
}
@@ -839,9 +1011,15 @@ bool dma2d_rgba8888_copy_mono8(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[1].InputAlpha = gfx_color_to_color32(bb->src_fg);
HAL_DMA2D_ConfigLayer(&drv->handle, 1);
- HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row + bb->src_x,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
return true;
}
@@ -879,10 +1057,17 @@ bool dma2d_rgba8888_copy_rgba8888(const gfx_bitblt_t* bb) {
drv->handle.LayerCfg[1].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 1);
- HAL_DMA2D_Start(&drv->handle,
- (uint32_t)bb->src_row + bb->src_x * sizeof(uint32_t),
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle,
+ (uint32_t)bb->src_row + bb->src_x * sizeof(uint32_t),
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ return false;
+ }
+
return true;
}
@@ -916,9 +1101,17 @@ static bool dma2d_rgba8888_copy_ycbcr(const gfx_bitblt_t* bb, uint32_t css) {
drv->handle.LayerCfg[1].InputAlpha = 0;
HAL_DMA2D_ConfigLayer(&drv->handle, 1);
- HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row,
- (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
- bb->width, bb->height);
+ if (HAL_OK ==
+ HAL_DMA2D_Start(&drv->handle, (uint32_t)bb->src_row,
+ (uint32_t)bb->dst_row + bb->dst_x * sizeof(uint32_t),
+ bb->width, bb->height)) {
+ drv->dma_transfer_in_progress = true;
+ } else {
+ dma2d_error_handler();
+ drv->clut_valid = false;
+
+ return false;
+ }
// DMA2D overwrites CLUT during YCbCr conversion
// (seems to be a bug or an undocumented feature)
Why this scored 28/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.