← Watch feed
chore(rust): improve unsafety annotations
Public commit record
57/100 · Thin
What the developer wrote
Authored by matejcik
chore(rust): improve unsafety annotations
in bootloader_c, prodtest_c
✓ 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 code-quality cleanup in the Rust portions of Trezor's bootloader and production-test interfaces. It marks C-callable functions as 'unsafe' (a Rust keyword meaning the caller must guarantee pointer validity) and adds explanatory comments. It also replaces a few raw pointer operations with a safer wrapper type. There is no direct evidence this fixes an active security bug, but it improves documentation and reduces risky raw-pointer use in firmware UI code that handles untrusted C inputs.
Recommended action
Treat as a low-risk hardening change. Review that all C callers of these functions actually validate pointers and lengths before calling, since the Rust side still relies on caller guarantees. No urgent action required absent additional evidence of a vulnerability.
Security signals we found
01
Unsafe FFI boundary between C and Rust in bootloader and prodtest UI code
02
Raw pointer dereferences and slice construction from caller-supplied pointers and lengths
03
Use of 'unsafe' keyword and SAFETY comments indicates awareness of memory-safety obligations
04
Replacement of raw slice construction with CSlice wrapper reduces direct unsafe surface
05
No explicit bug fix, CVE, or security disclosure referenced in commit
Evidence from the diff
The diff updates FFI functions in bootloader_c.rs and prodtest_c.rs to be declared ‘unsafe extern “C”’ and adds SAFETY comments. It replaces some core::slice::from_raw_parts calls with CSlice::from_ptr_and_len, and removes redundant ‘as usize’ casts. In screen_prodtest_welcome, the unbounded ASCII string conversion is moved earlier. These are defensive improvements to unsafe Rust annotations; they do not change the underlying trust model (caller still must provide valid pointers/lengths).
Changed components
core/embed/rust/src/ui/api/bootloader_c.rscore/embed/rust/src/ui/api/prodtest_c.rsTrezor bootloader UI FFI layerTrezor production-test UI FFI layerInspect captured patch +45 / −39
### core/embed/rust/src/ui/api/bootloader_c.rs
@@ -5,11 +5,10 @@ use crate::ui::ui_bootloader::BootloaderUI;
use crate::ui::ModelUI;
#[no_mangle]
-extern "C" fn screen_welcome(ui_action_result: *mut u32) -> u32 {
+unsafe extern "C" fn screen_welcome(ui_action_result: *mut u32) -> u32 {
let (res, ui_res) = ModelUI::screen_welcome();
- unsafe {
- *ui_action_result = ui_res;
- }
+ // SAFETY: caller should provide a valid pointer
+ unsafe { *ui_action_result = ui_res };
res
}
@@ -28,7 +27,7 @@ extern "C" fn screen_install_fail() {
}
#[no_mangle]
-extern "C" fn screen_install_confirm(
+unsafe extern "C" fn screen_install_confirm(
vendor_str: *const cty::c_char,
vendor_str_len: u8,
version: *const cty::c_char,
@@ -38,6 +37,7 @@ extern "C" fn screen_install_confirm(
is_newinstall: bool,
version_cmp: cty::c_int,
) -> u32 {
+ // SAFETY: caller should provide valid pointers
let text = unsafe { CSlice::from_ptr_and_len(vendor_str, vendor_str_len as usize) };
let version = unsafe { CSlice::from_c_str(version) };
@@ -75,26 +75,26 @@ extern "C" fn screen_unlock_bootloader_success() {
}
#[no_mangle]
-extern "C" fn screen_menu(
+unsafe extern "C" fn screen_menu(
initial_setup: bool,
communication: bool,
ui_action_result: *mut u32,
) -> u32 {
let (res, ui_res) = ModelUI::screen_menu(initial_setup, communication);
- unsafe {
- *ui_action_result = ui_res;
- }
+ // SAFETY: caller should provide a valid pointer
+ unsafe { *ui_action_result = ui_res };
res
}
#[no_mangle]
-extern "C" fn screen_intro(
+unsafe extern "C" fn screen_intro(
bld_version: *const cty::c_char,
vendor_str: *const cty::c_char,
vendor_str_len: u8,
version: *const cty::c_char,
fw_ok: bool,
) -> u32 {
+ // SAFETY: caller should provide valid pointers
let vendor = unsafe { CSlice::from_ptr_and_len(vendor_str, vendor_str_len as usize) };
let version = unsafe { CSlice::from_c_str(version) };
let bld_version = unsafe { CSlice::from_c_str(bld_version) };
@@ -118,7 +118,7 @@ extern "C" fn screen_boot_empty() {
}
#[no_mangle]
-extern "C" fn screen_boot(
+unsafe extern "C" fn screen_boot(
warning: bool,
vendor_str: *const cty::c_char,
vendor_str_len: usize,
@@ -127,11 +127,9 @@ extern "C" fn screen_boot(
vendor_img_len: usize,
wait: i32,
) {
- let vendor_str = unsafe { CSlice::from_ptr_and_len(vendor_str, vendor_str_len as usize) };
- // vendor_img MUST be a pointer to 'static memory, which is why we're
- // sidestepping CSlice's rules
- let vendor_img =
- unsafe { core::slice::from_raw_parts(vendor_img as *const u8, vendor_img_len) };
+ // SAFETY: caller should provide valid pointers
+ let vendor_str = unsafe { CSlice::from_ptr_and_len(vendor_str, vendor_str_len) };
+ let vendor_img = unsafe { CSlice::from_ptr_and_len(vendor_img as *const u8, vendor_img_len) };
// Splits a version stored as a u32 into four numbers
// starting with the major version.
@@ -141,7 +139,7 @@ extern "C" fn screen_boot(
warning,
vendor_str.as_ascii_str(),
version,
- vendor_img,
+ vendor_img.as_slice().unwrap_or_default(),
wait,
)
}
@@ -168,9 +166,8 @@ extern "C" fn screen_connect(
ui_action_result: *mut u32,
) -> u32 {
let (res, ui_res) = ModelUI::screen_connect(initial_setup, show_menu);
- unsafe {
- *ui_action_result = ui_res;
- }
+ // SAFETY: caller should provide a valid pointer
+ unsafe { *ui_action_result = ui_res };
res
}
@@ -192,35 +189,35 @@ extern "C" fn screen_confirm_pairing(code: u32, initial_setup: bool) -> u32 {
#[cfg(feature = "ble")]
#[no_mangle]
-extern "C" fn screen_pairing_mode(
+unsafe extern "C" fn screen_pairing_mode(
initial_setup: bool,
name: *const cty::c_char,
name_len: usize,
ui_action_result: *mut u32,
) -> u32 {
- let name = unsafe { CSlice::from_ptr_and_len(name, name_len as usize) };
+ // SAFETY: caller should provide a valid string
+ let name = unsafe { CSlice::from_ptr_and_len(name, name_len) };
let (res, ui_res) =
ModelUI::screen_pairing_mode(initial_setup, name.as_ascii_str().unwrap_or_default());
- unsafe {
- *ui_action_result = ui_res;
- }
+ // SAFETY: caller should provide a valid pointer
+ unsafe { *ui_action_result = ui_res };
res
}
#[cfg(feature = "ble")]
#[no_mangle]
-extern "C" fn screen_wireless_setup(
+unsafe extern "C" fn screen_wireless_setup(
name: *const cty::c_char,
name_len: usize,
ui_action_result: *mut u32,
) -> u32 {
- let name = unsafe { CSlice::from_ptr_and_len(name, name_len as usize) };
+ // SAFETY: caller should provide a valid string
+ let name = unsafe { CSlice::from_ptr_and_len(name, name_len) };
let (res, ui_res) = ModelUI::screen_wireless_setup(name.as_ascii_str().unwrap_or_default());
- unsafe {
- *ui_action_result = ui_res;
- }
+ // SAFETY: caller should provide a valid pointer
+ unsafe { *ui_action_result = ui_res };
res
}
### core/embed/rust/src/ui/api/prodtest_c.rs
@@ -14,7 +14,10 @@ use crate::ui::ModelUI;
use crate::ui::{event::TouchEvent, layout::simplified::touch_unpack};
#[no_mangle]
-extern "C" fn screen_prodtest_event(layout: *mut c_layout_t, signalled: &sysevents_t) -> u32 {
+unsafe extern "C" fn screen_prodtest_event(
+ layout: *mut c_layout_t,
+ signalled: &sysevents_t,
+) -> u32 {
let e = parse_event(signalled);
// SAFETY: calling code is supposed to give us exclusive access to an already
// initialized layout
@@ -26,12 +29,17 @@ extern "C" fn screen_prodtest_event(layout: *mut c_layout_t, signalled: &syseven
}
#[no_mangle]
-extern "C" fn screen_prodtest_welcome(layout: *mut c_layout_t, id: *const cty::c_char, id_len: u8) {
- let id = unsafe { CSlice::from_ptr_and_len(id, id_len as usize) };
+unsafe extern "C" fn screen_prodtest_welcome(
+ layout: *mut c_layout_t,
+ id: *const cty::c_char,
+ id_len: u8,
+) {
+ // SAFETY: caller must provide a pointer to `id` that has the same effective
+ // lifetime as `layout`. Then we use `into_unbounded_ascii_str` so that we
+ // can pass the resulting reference for storage in `layout`
+ let id = unsafe { CSlice::from_ptr_and_len(id, id_len as usize).into_unbounded_ascii_str() };
- let mut screen = <ModelUI as ProdtestUI>::CLayoutType::init_welcome(unsafe {
- id.into_unbounded_ascii_str()
- });
+ let mut screen = <ModelUI as ProdtestUI>::CLayoutType::init_welcome(id);
screen.show();
// SAFETY: calling code is supposed to give us exclusive access to the layout
let mut layout = unsafe { LayoutBuffer::new(layout) };
@@ -65,12 +73,13 @@ extern "C" fn screen_prodtest_touch(x0: int16_t, y0: int16_t, w: int16_t, h: int
#[no_mangle]
#[cfg(feature = "touch")]
-extern "C" fn screen_prodtest_draw(events: *const cty::uint32_t, events_len: u32) {
- let events = unsafe { core::slice::from_raw_parts(events, events_len as usize) };
+unsafe extern "C" fn screen_prodtest_draw(events: *const cty::uint32_t, events_len: u32) {
+ // SAFETY: caller must provide a valid pointer
+ let events = unsafe { CSlice::from_ptr_and_len(events, events_len as usize) };
let mut v: Vec<TouchEvent, 256> = Vec::new();
- for e in events.iter() {
+ for e in events.as_slice().unwrap_or_default().iter() {
if let Some(event) = touch_unpack(*e) {
unwrap!(v.push(event));
}Risk score
Our methodology →Why this scored 20/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.