refactor(core): the display driver's usage from RUST code has been adapted to the modified C driver code. The "backlight" function has been divided into "set_backlight" and "get_backlight" ftuncions. The data types respect the ones from C code. The "set_backlight" function parameter "value" (level
What the developer wrote
Authored by bleska
refactor(core): the display driver's usage from RUST code has been adapted to the modified C driver code. The "backlight" function has been divided into "set_backlight" and "get_backlight" ftuncions. The data types respect the ones from C code. The "set_backlight" function parameter "value" (level of backlight) has data type uint8 (u8). Some function call, however, use the u16 data type for this argument. It's necessary to review whether there isn't any problem with this.
[no changelog]
What changed, and why it matters
This commit refactors how the Trezor hardware wallet's screen brightness is controlled from Rust code. The old single 'backlight' function is split into separate 'set_backlight' and 'get_backlight' functions, and the brightness value type is narrowed from a wider signed integer to an unsigned 8-bit value (0-255). The commit message and code comments explicitly flag that some callers still pass a 16-bit value, and the patch adds defensive clamping to 255 in those cases. There is no direct evidence of an exploitable security bug, but the change is clearly defensive hardening against potential type-conversion issues.
Treat as a hardening/refactor commit that partially addresses a type-safety concern. Review the remaining layout_delizia 'n as _' cast and any other u16 brightness sources to ensure values cannot exceed u8::MAX (255) before reaching the driver. Consider adding explicit bounds checks or changing the slider value type to u8 across all layouts. No immediate security advisory is warranted based solely on this diff, but the flagged TODOs should be resolved.
Security signals we found
Type narrowing from i32/u16 to u8 for hardware driver parameter
Defensive clamping added where caller type (u16) exceeds callee type (u8)
Commit message explicitly questions safety of remaining u16 -> u8 conversions
TODO comments in code flag unresolved type-safety analysis
One call site (layout_delizia) still casts u16 to u8 with 'as _' rather than clamping
No changelog entry despite acknowledged potential safety issue
Evidence from the diff
The patch updates Rust FFI bindings and UI code to match a C driver change: display::backlight(i32) -> i32 is replaced by display_set_backlight(u8) -> bool and display_get_backlight() -> u8. Rust wrappers in ui/display/mod.rs and trezorhal/display.rs are updated accordingly. Two call sites (layout_bolt SetBrightnessDialog and layout_eckhart VerticalSlider) previously passed u16 values via .into(); they now use try_into() and clamp to 255 on failure. A third call site (layout_delizia) still uses ‘n as _’ to cast to u8. The commit message notes the u16/u8 mismatch and requests further review. No changelog entry is recorded.
Changed components
core/embed/rust/src/trezorhal/display.rscore/embed/rust/src/ui/display/mod.rscore/embed/rust/src/ui/api/firmware_micropython.rscore/embed/rust/src/ui/layout/simplified.rscore/embed/rust/src/ui/layout_bolt/component/set_brightness.rscore/embed/rust/src/ui/layout_delizia/flow/set_brightness.rscore/embed/rust/src/ui/layout_eckhart/firmware/brightness_screen.rsInspect captured patch +33 / −11
diff --git a/core/embed/rust/src/trezorhal/display.rs b/core/embed/rust/src/trezorhal/display.rs
index c88958c6..ad65ecc3 100644
--- a/core/embed/rust/src/trezorhal/display.rs
+++ b/core/embed/rust/src/trezorhal/display.rs
@@ -8,10 +8,14 @@ use ffi::{DISPLAY_RESX_, DISPLAY_RESY_};
pub const DISPLAY_RESX: u32 = DISPLAY_RESX_;
pub const DISPLAY_RESY: u32 = DISPLAY_RESY_;
-pub fn backlight(val: i32) -> i32 {
+pub fn set_backlight(val: u8) -> bool {
unsafe { ffi::display_set_backlight(val) }
}
+pub fn get_backlight() -> u8 {
+ unsafe { ffi::display_get_backlight() }
+}
+
pub fn sync() {
// NOTE: The sync operation is not called for tests because the linker
// would otherwise report missing symbols if the tests are built with ASAN.
diff --git a/core/embed/rust/src/ui/api/firmware_micropython.rs b/core/embed/rust/src/ui/api/firmware_micropython.rs
index 24827a0b..070af6f9 100644
--- a/core/embed/rust/src/ui/api/firmware_micropython.rs
+++ b/core/embed/rust/src/ui/api/firmware_micropython.rs
@@ -35,7 +35,7 @@ use crate::{
use heapless::Vec;
#[cfg(feature = "backlight")]
-use crate::ui::display::{backlight, fade_backlight_duration, set_backlight};
+use crate::ui::display::{fade_backlight_duration, get_backlight, set_backlight};
/// Dummy implementation so that we can use `Empty` in a return type of
/// unimplemented trait function
@@ -1319,7 +1319,7 @@ pub extern "C" fn upy_backlight_get() -> Obj {
let block = || {
#[cfg(feature = "backlight")]
{
- let backlight_level = backlight();
+ let backlight_level = get_backlight();
Ok(Obj::from(backlight_level))
}
#[cfg(not(feature = "backlight"))]
diff --git a/core/embed/rust/src/ui/display/mod.rs b/core/embed/rust/src/ui/display/mod.rs
index efe03a6b..17027247 100644
--- a/core/embed/rust/src/ui/display/mod.rs
+++ b/core/embed/rust/src/ui/display/mod.rs
@@ -24,13 +24,14 @@ pub use font::{Font, Glyph, GlyphMetrics};
pub const LOADER_MIN: u16 = 0;
pub const LOADER_MAX: u16 = 1000;
-pub fn backlight() -> u8 {
- display::backlight(-1) as u8
+#[cfg(feature = "backlight")]
+pub fn get_backlight() -> u8 {
+ display::get_backlight()
}
#[cfg(feature = "backlight")]
pub fn set_backlight(val: u8) {
- display::backlight(val as i32);
+ display::set_backlight(val);
}
#[cfg(feature = "backlight")]
@@ -41,7 +42,7 @@ pub fn fade_backlight(target: u8) {
#[cfg(feature = "backlight")]
pub fn fade_backlight_duration(target: u8, duration_ms: u32) {
- let current = backlight();
+ let current = get_backlight();
let duration = Duration::from_millis(duration_ms);
if animation_disabled() {
diff --git a/core/embed/rust/src/ui/layout/simplified.rs b/core/embed/rust/src/ui/layout/simplified.rs
index ab29f87e..db7bdc60 100644
--- a/core/embed/rust/src/ui/layout/simplified.rs
+++ b/core/embed/rust/src/ui/layout/simplified.rs
@@ -252,7 +252,8 @@ pub fn show(frame: &mut impl Component<Msg = impl ReturnToC>, fading: bool) -> u
return message.return_to_c();
}
- if fading && display::backlight() > 0 {
+ #[cfg(feature = "backlight")]
+ if fading && display::get_backlight() > 0 {
ModelUI::fadeout()
};
diff --git a/core/embed/rust/src/ui/layout_bolt/component/set_brightness.rs b/core/embed/rust/src/ui/layout_bolt/component/set_brightness.rs
index 5b74b845..3f9075ae 100644
--- a/core/embed/rust/src/ui/layout_bolt/component/set_brightness.rs
+++ b/core/embed/rust/src/ui/layout_bolt/component/set_brightness.rs
@@ -36,7 +36,15 @@ impl Component for SetBrightnessDialog {
fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option<Self::Msg> {
match self.0.event(ctx, event) {
Some(NumberInputSliderDialogMsg::Changed(value)) => {
- display::backlight(value.into());
+ // TODO: needs more analysis; why is "value" u16? Can it be changed to u8?
+ // Original code: display::set_backlight(value.into());
+ // Possible solution: display::set_backlight(value.try_into().unwrap());
+ // It's not save. To rather use unwrap!() macro? Another solution below.
+ if let Ok(val) = value.try_into() {
+ display::set_backlight(val);
+ } else {
+ display::set_backlight(255);
+ }
None
}
Some(NumberInputSliderDialogMsg::Cancelled) => Some(CancelConfirmMsg::Cancelled),
diff --git a/core/embed/rust/src/ui/layout_delizia/flow/set_brightness.rs b/core/embed/rust/src/ui/layout_delizia/flow/set_brightness.rs
index 9229a517..b4a64c46 100644
--- a/core/embed/rust/src/ui/layout_delizia/flow/set_brightness.rs
+++ b/core/embed/rust/src/ui/layout_delizia/flow/set_brightness.rs
@@ -87,7 +87,7 @@ pub fn new_set_brightness(brightness: u8) -> Result<SwipeFlow, Error> {
.register_footer_update_fn(footer_update_fn)
.map(|msg| match msg {
NumberInputSliderDialogMsg::Changed(n) => {
- display::backlight(n as _);
+ display::set_backlight(n as _);
BRIGHTNESS.store(n as u8, Ordering::Relaxed);
None
}
diff --git a/core/embed/rust/src/ui/layout_eckhart/firmware/brightness_screen.rs b/core/embed/rust/src/ui/layout_eckhart/firmware/brightness_screen.rs
index 8499118b..59598a54 100644
--- a/core/embed/rust/src/ui/layout_eckhart/firmware/brightness_screen.rs
+++ b/core/embed/rust/src/ui/layout_eckhart/firmware/brightness_screen.rs
@@ -113,7 +113,15 @@ impl VerticalSlider {
fn handle_touch(&mut self, pos: Point, ctx: &mut EventCtx) {
self.update_value(pos, ctx);
- display::backlight(self.value.into());
+ // TODO: needs more analysis; why is "self.value" u16? Can it be changed to u8?
+ // Original code: display::set_backlight(self.value.into());
+ // Possible solution: display::set_backlight(self.value.try_into().unwrap());
+ // It's not save. To rather use unwrap!() macro? Another solution below.
+ if let Ok(val) = self.value.try_into() {
+ display::set_backlight(val);
+ } else {
+ display::set_backlight(255);
+ }
ctx.request_paint();
}
Why this scored 22/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.