← Watch feed
feat(core): improve dbg_console interface
Public commit record
57/100 · Thin
What the developer wrote
Authored by cepetr
feat(core): improve dbg_console interface
[no changelog]
✓ Descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope! No meaningful explanatory body
What changed, and why it matters
This commit is a small internal cleanup of the Trezor firmware's debug console. It changes the debug-console write function so it returns the number of bytes written (or an error code) instead of returning nothing. It also makes the USB virtual-com-port backend avoid blocking when called from interrupt context. The changes are confined to debug-only code and do not appear to fix a known security vulnerability.
Recommended action
No immediate security action is required. Treat this as a normal code-quality/debug-interface improvement. Reviewers may want to confirm that callers of `dbg_console_write()` now correctly handle the new return value, especially in error paths, but the change itself is defensive and low risk.
Security signals we found
01
Function signature change from void to ssize_t with return-value propagation
02
Interrupt-context safety adjustment: VCP write timeout set to 0 when not in thread/SVCall mode
03
No input validation, bounds checking, or privilege changes observed
04
No changelog entry; commit is described as an interface improvement
Evidence from the diff
The patch refactors dbg_console_write() from void to ssize_t, propagating the return value through the STM32 syscall dispatch, syscall stubs, and verifier wrappers. The STM32 backend now returns byte counts from its SWO, SystemView, and USB VCP helpers. In the VCP backend, BLOCK_ON_VCP logic was adjusted to use __get_IPSR() to detect thread vs. interrupt/SVCall mode and set a 1000 ms timeout only in thread mode, avoiding blocking in interrupt context. The Unix backend now returns the result of write(STDERR_FILENO, ...) instead of discarding it. No input validation, memory safety, or cryptographic changes are present.
Changed components
core/embed/sys/dbg/inc/sys/dbg_console.hcore/embed/sys/dbg/stm32/dbg_console_backend.ccore/embed/sys/dbg/unix/dbg_console_backend.ccore/embed/sys/syscall/stm32/syscall_dispatch.ccore/embed/sys/syscall/stm32/syscall_stubs.ccore/embed/sys/syscall/stm32/syscall_verifiers.ccore/embed/sys/syscall/stm32/syscall_verifiers.hInspect captured patch +35 / −20
diff --git a/core/embed/sys/dbg/inc/sys/dbg_console.h b/core/embed/sys/dbg/inc/sys/dbg_console.h
index 4be29f55e..649c862b5 100644
--- a/core/embed/sys/dbg/inc/sys/dbg_console.h
+++ b/core/embed/sys/dbg/inc/sys/dbg_console.h
@@ -49,10 +49,15 @@ ssize_t dbg_console_read(void* buffer, size_t buffer_size);
/**
* @brief Write data to the debugging console.
*
+ * The function may be blocking, depending on the backend implementation
+ * and its configuration. If called from interrupt context, the function
+ * is always non-blocking.
+ *
* @param data Pointer to the data to write.
* @param data_size Size of the data in bytes.
+ * @return Number of bytes written, or a negative error code on failure.
*/
-void dbg_console_write(const void* data, size_t data_size);
+ssize_t dbg_console_write(const void* data, size_t data_size);
/**
* @brief vprintf-like function for debugging.
@@ -65,6 +70,9 @@ void dbg_console_vprintf(const char* fmt, va_list args);
/**
* @brief printf-like function for debugging.
*
+ * If possible, consider using `LOG_xxx()` macro instead of this function.
+ * These macros provide standardized message formatting and filtering.
+ *
* @param fmt Format string.
* @param ... Variable arguments.
*/
diff --git a/core/embed/sys/dbg/stm32/dbg_console_backend.c b/core/embed/sys/dbg/stm32/dbg_console_backend.c
index b29627041..105f79177 100644
--- a/core/embed/sys/dbg/stm32/dbg_console_backend.c
+++ b/core/embed/sys/dbg/stm32/dbg_console_backend.c
@@ -49,7 +49,7 @@ void dbg_console_init(void) {
ssize_t dbg_console_read(void *buffer, size_t buffer_size) { return 0; }
#ifdef USE_DBG_CONSOLE_SWO
-static void itm_swo_write(const void *data, size_t data_size) {
+static ssize_t itm_swo_write(const void *data, size_t data_size) {
irq_key_t irq_key = irq_lock();
for (size_t i = 0; i < data_size; i++) {
@@ -57,11 +57,12 @@ static void itm_swo_write(const void *data, size_t data_size) {
}
irq_unlock(irq_key);
+ return data_size;
}
#endif
#ifdef USE_DBG_CONSOLE_SYSTEM_VIEW
-static void sysview_write(const void *data, size_t data_size) {
+static ssize_t sysview_write(const void *data, size_t data_size) {
#if 1
static char str[512];
strncpy(str, (const char *)data, sizeof(str) - 1);
@@ -71,29 +72,36 @@ static void sysview_write(const void *data, size_t data_size) {
#if 0
SEGGER_RTT_Write(0, data, data_size);
#endif
+ return MIN(data_size, sizeof(str) - 1);
}
#endif
#ifdef USE_DBG_CONSOLE_VCP
-static void usb_vcp_write(const void *data, size_t data_size) {
+static ssize_t usb_vcp_write(const void *data, size_t data_size) {
#ifdef BLOCK_ON_VCP
- syshandle_write_blocking(SYSHANDLE_USB_VCP, data, data_size, 1000);
+ // In thread mode, we can wait for the VCP to be ready.
+ // In interrupt context, we must not block.
+ uint32_t ipsr = __get_IPSR();
+ bool thread_mode = (ipsr == 0 || ipsr == 11); // Thread mode or SVCall
+ uint32_t timeout = thread_mode ? 1000 : 0;
+ return syshandle_write_blocking(SYSHANDLE_USB_VCP, data, data_size, timeout);
#else
- syshandle_write(SYSHANDLE_USB_VCP, data, data_size);
+ return syshandle_write(SYSHANDLE_USB_VCP, data, data_size);
#endif
}
#endif
-void dbg_console_write(const void *data, size_t data_size) {
+ssize_t dbg_console_write(const void *data, size_t data_size) {
#ifdef USE_DBG_CONSOLE_SWO
- itm_swo_write(data, data_size);
+ return itm_swo_write(data, data_size);
#endif
#ifdef USE_DBG_CONSOLE_SYSTEM_VIEW
- sysview_write(data, data_size);
+ return sysview_write(data, data_size);
#endif
#ifdef USE_DBG_CONSOLE_VCP
- usb_vcp_write(data, data_size);
+ return usb_vcp_write(data, data_size);
#endif
+ return -1;
}
#endif // KERNEL_MODE
diff --git a/core/embed/sys/dbg/unix/dbg_console_backend.c b/core/embed/sys/dbg/unix/dbg_console_backend.c
index 85c0e747e..21dcfd2dc 100644
--- a/core/embed/sys/dbg/unix/dbg_console_backend.c
+++ b/core/embed/sys/dbg/unix/dbg_console_backend.c
@@ -26,7 +26,6 @@ void dbg_console_init(void) {}
ssize_t dbg_console_read(void *buffer, size_t buffer_size) { return 0; }
-void dbg_console_write(const void *data, size_t data_size) {
- int result = write(STDERR_FILENO, data, data_size);
- (void)result;
+ssize_t dbg_console_write(const void *data, size_t data_size) {
+ return write(STDERR_FILENO, data, data_size);
}
diff --git a/core/embed/sys/syscall/stm32/syscall_dispatch.c b/core/embed/sys/syscall/stm32/syscall_dispatch.c
index 62963537f..a2fbaea5d 100644
--- a/core/embed/sys/syscall/stm32/syscall_dispatch.c
+++ b/core/embed/sys/syscall/stm32/syscall_dispatch.c
@@ -181,7 +181,7 @@ __attribute((no_stack_protector)) void syscall_handler(uint32_t *args,
case SYSCALL_DBG_CONSOLE_WRITE: {
const void *data = (const void *)args[0];
size_t data_size = (size_t)args[1];
- dbg_console_write__verified(data, data_size);
+ args[0] = dbg_console_write__verified(data, data_size);
} break;
#endif
diff --git a/core/embed/sys/syscall/stm32/syscall_stubs.c b/core/embed/sys/syscall/stm32/syscall_stubs.c
index ae794e8c3..4cb79fd12 100644
--- a/core/embed/sys/syscall/stm32/syscall_stubs.c
+++ b/core/embed/sys/syscall/stm32/syscall_stubs.c
@@ -108,8 +108,8 @@ ssize_t dbg_console_read(void *buffer, size_t buffer_size) {
SYSCALL_DBG_CONSOLE_READ);
}
-void dbg_console_write(const void *data, size_t data_size) {
- syscall_invoke2((uint32_t)data, data_size, SYSCALL_DBG_CONSOLE_WRITE);
+ssize_t dbg_console_write(const void *data, size_t data_size) {
+ return syscall_invoke2((uint32_t)data, data_size, SYSCALL_DBG_CONSOLE_WRITE);
}
#endif // USE_DBG_CONSOLE
diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.c b/core/embed/sys/syscall/stm32/syscall_verifiers.c
index 70857d50d..e5f13b105 100644
--- a/core/embed/sys/syscall/stm32/syscall_verifiers.c
+++ b/core/embed/sys/syscall/stm32/syscall_verifiers.c
@@ -104,16 +104,16 @@ access_violation:
return -1;
}
-void dbg_console_write__verified(const void *data, size_t data_size) {
+ssize_t dbg_console_write__verified(const void *data, size_t data_size) {
if (!probe_read_access(data, data_size)) {
goto access_violation;
}
- dbg_console_write(data, data_size);
- return;
+ return dbg_console_write(data, data_size);
access_violation:
apptask_access_violation();
+ return -1;
}
#endif // USE_DBG_CONSOLE
diff --git a/core/embed/sys/syscall/stm32/syscall_verifiers.h b/core/embed/sys/syscall/stm32/syscall_verifiers.h
index 350828b49..43ea17a83 100644
--- a/core/embed/sys/syscall/stm32/syscall_verifiers.h
+++ b/core/embed/sys/syscall/stm32/syscall_verifiers.h
@@ -52,7 +52,7 @@ void system_exit_fatal__verified(const char *message, size_t message_len,
ssize_t dbg_console_read__verified(void *buffer, size_t buffer_size);
-void dbg_console_write__verified(const void *data, size_t data_size);
+ssize_t dbg_console_write__verified(const void *data, size_t data_size);
#endif
Risk score
Our methodology →Why this scored 18/100
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.