refactor(core): remove circulars deps between util and sys

Trezor/Trezor firmware

dc48609b View source ↗

Public commit record

What the developer wrote

Authored by cepetr

62/100 · Adequate

refactor(core): remove circulars deps between util and sys

[no changelog]

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body

The short version

What changed, and why it matters

This is a code cleanup change that removes circular dependencies between low-level system modules and image-handling utilities. It replaces a few shared macro definitions with local equivalents and adds compile-time checks to ensure the duplicated constants stay in sync. There is no indication this fixes a security bug or changes device behavior in a security-relevant way.

Recommended action

No security action required; treat as routine refactoring. Standard code-review verification that the new _Static_asserts and macro replacements preserve existing alignment and storage layout is sufficient.

Security signals we found

No strong security signals were identified.

Technical analysis

Evidence from the diff

The commit refactors embedded firmware build dependencies. It removes includes of util/flash.h and util/image.h from sys/ mpu, startup, and trustzone code, and removes util/image.h from coreapp.c. To avoid breaking functionality, it duplicates the bootloader vector-table offset (IMAGE_HEADER_SIZE) as BOOTLOADER_VECTBL_OFFSET in sys/bootutils.h and adds a _Static_assert in image.c verifying the two values match. It also replaces the COREAPP_CODE_ALIGN macro (from util/image.h) with the standard ALIGN_UP macro from rtl/sizedefs.h, and redefines STORAGE_SIZE in the MPU code using STORAGE_1_MAXSIZE + STORAGE_2_MAXSIZE plus a contiguity assertion. These are build-time/dependency refactorings with no functional security change evident from the diff.

Changed components

core/embed/sys/mpu/stm32u5/mpu.ccore/embed/sys/startup/inc/sys/bootutils.hcore/embed/sys/startup/stm32/bootutils.ccore/embed/sys/task/stm32/coreapp.ccore/embed/sys/trustzone/stm32u5/trustzone.ccore/embed/util/image/image.ccore/embed/util/image/inc/util/image.h

Inspect captured patch +22 / −14

diff --git a/core/embed/sys/mpu/stm32u5/mpu.c b/core/embed/sys/mpu/stm32u5/mpu.c
index 4f058996a..4ed94d148 100644
--- a/core/embed/sys/mpu/stm32u5/mpu.c
+++ b/core/embed/sys/mpu/stm32u5/mpu.c
@@ -30,8 +30,6 @@
 #include <rtl/sizedefs.h>
 #include <sys/irq.h>
 #include <sys/mpu.h>
-#include <util/flash.h>
-#include <util/image.h>
 
 #include "stm32u5xx_ll_cortex.h"
 
@@ -128,7 +126,10 @@ static void mpu_set_attributes(void) {
   MPU->MAIR0 |= 0x44 << 24;
 }
 
-#define STORAGE_SIZE (NORCOW_SECTOR_SIZE * NORCOW_SECTOR_COUNT)
+#define STORAGE_SIZE (STORAGE_1_MAXSIZE + STORAGE_2_MAXSIZE)
+_Static_assert(STORAGE_1_START + STORAGE_1_MAXSIZE == STORAGE_2_START,
+               "storage regions not contiguous");
+
 _Static_assert(NORCOW_SECTOR_SIZE == STORAGE_1_MAXSIZE, "norcow misconfigured");
 _Static_assert(NORCOW_SECTOR_SIZE == STORAGE_2_MAXSIZE, "norcow misconfigured");
 
@@ -178,7 +179,7 @@ extern uint32_t _kernel_flash_end;
 #define KERNEL_START FIRMWARE_START
 #endif
 
-#define KERNEL_END COREAPP_CODE_ALIGN((uint32_t) & _kernel_flash_end)
+#define KERNEL_END ALIGN_UP((uint32_t) & _kernel_flash_end, COREAPP_ALIGNMENT)
 #define KERNEL_SIZE (KERNEL_END - KERNEL_START)
 #endif  // KERNEL
 
diff --git a/core/embed/sys/startup/inc/sys/bootutils.h b/core/embed/sys/startup/inc/sys/bootutils.h
index 130d006eb..8498e9394 100644
--- a/core/embed/sys/startup/inc/sys/bootutils.h
+++ b/core/embed/sys/startup/inc/sys/bootutils.h
@@ -21,6 +21,13 @@
 
 #include <sys/systask.h>
 
+#ifdef STM32F4
+// Offset of the vector table in the bootloader image.
+// This should match IMAGE_HEADER_SIZE in util/image.h.
+// Duplicated here to avoid circular dependency.
+#define BOOTLOADER_VECTBL_OFFSET 0x400
+#endif
+
 // Wipe information structure
 typedef struct {
   char title[64];
diff --git a/core/embed/sys/startup/stm32/bootutils.c b/core/embed/sys/startup/stm32/bootutils.c
index 675cd07c8..9e5043ae8 100644
--- a/core/embed/sys/startup/stm32/bootutils.c
+++ b/core/embed/sys/startup/stm32/bootutils.c
@@ -31,7 +31,6 @@
 #include <sys/stack_utils.h>
 #include <sys/systick.h>
 #include <sys/sysutils.h>
-#include <util/image.h>
 
 #ifdef STM32F4
 #include <io/display.h>
@@ -175,7 +174,7 @@ static void reboot_with_args_phase_2(uint32_t arg1, uint32_t arg2) {
     SysTick_Config(HAL_RCC_GetSysClockFreq() / 1000U);
     NVIC_SetPriority(SysTick_IRQn, 0);
 #endif
-    jump_to_vectbl(BOOTLOADER_START + IMAGE_HEADER_SIZE, command);
+    jump_to_vectbl(BOOTLOADER_START + BOOTLOADER_VECTBL_OFFSET, command);
   }
 #else
 #error Unsupported platform
diff --git a/core/embed/sys/task/stm32/coreapp.c b/core/embed/sys/task/stm32/coreapp.c
index c3764471c..764331bd6 100644
--- a/core/embed/sys/task/stm32/coreapp.c
+++ b/core/embed/sys/task/stm32/coreapp.c
@@ -19,21 +19,22 @@
 
 #ifdef KERNEL
 
+#include <trezor_model.h>
 #include <trezor_rtl.h>
 
+#include <rtl/sizedefs.h>
 #include <sec/rng.h>
 #include <sys/applet.h>
 #include <sys/coreapp.h>
 #include <sys/mpu.h>
 #include <sys/systask.h>
-#include <util/image.h>
 
 static mpu_area_t coreapp_code_area;
 static mpu_area_t coreapp_tls_area;
 
 // defined in linker script
 extern uint32_t _kernel_flash_end;
-#define KERNEL_END COREAPP_CODE_ALIGN((uint32_t) & _kernel_flash_end)
+#define KERNEL_END ALIGN_UP((uint32_t) & _kernel_flash_end, COREAPP_ALIGNMENT)
 
 // Initializes coreapp applet
 void coreapp_init(applet_t* applet) {
diff --git a/core/embed/sys/trustzone/stm32u5/trustzone.c b/core/embed/sys/trustzone/stm32u5/trustzone.c
index 62c1bd39e..a2e48bf6f 100644
--- a/core/embed/sys/trustzone/stm32u5/trustzone.c
+++ b/core/embed/sys/trustzone/stm32u5/trustzone.c
@@ -26,8 +26,6 @@
 #include <rtl/sizedefs.h>
 #include <sys/irq.h>
 #include <sys/trustzone.h>
-#include <util/flash.h>
-#include <util/image.h>
 
 #if defined(__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U)
 
diff --git a/core/embed/util/image/image.c b/core/embed/util/image/image.c
index c417e596d..5d03214c3 100644
--- a/core/embed/util/image/image.c
+++ b/core/embed/util/image/image.c
@@ -22,9 +22,15 @@
 
 #include "ed25519-donna/ed25519.h"
 
+#include <sys/bootutils.h>
 #include <util/flash.h>
 #include <util/image.h>
 
+#ifdef STM32F4
+_Static_assert(BOOTLOADER_VECTBL_OFFSET == IMAGE_HEADER_SIZE,
+               "BOOTLOADER_VECTBL_OFFSET must match IMAGE_HEADER_SIZE");
+#endif
+
 _Static_assert(VENDOR_HEADER_MAX_SIZE + IMAGE_HEADER_SIZE <= IMAGE_CHUNK_SIZE,
                "The size of the firmware headers must be less than or equal to "
                "IMAGE_CHUNK_SIZE");
diff --git a/core/embed/util/image/inc/util/image.h b/core/embed/util/image/inc/util/image.h
index 3b8e85c05..6b7e3cb5c 100644
--- a/core/embed/util/image/inc/util/image.h
+++ b/core/embed/util/image/inc/util/image.h
@@ -39,10 +39,6 @@
 
 #define SECMON_IMAGE_MAGIC 0x43455354  // TSEC
 
-#define COREAPP_CODE_ALIGN(addr)                             \
-  ((((uint32_t)(uintptr_t)addr) + (COREAPP_ALIGNMENT - 1)) & \
-   ~(COREAPP_ALIGNMENT - 1))
-
 typedef struct {
   uint32_t magic;
   uint32_t hdrlen;

Risk score

Why this scored 12/100

Our methodology →

Potential impact 0/30

Exploitability 0/25

Stealth signal 0/15

Affected reach 0/15

Confidence 8/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 12/100

Community notes

Related watches

refactor(core): digest EIP-1559/EIP-7702 magic without RLP encoding

test(core): test RLP self-encoding bytestrings

chore(core): remove sha1 from regular FW