refactor(core/rust): don't panic if Rust layout is unavailable
What changed, and why it matters
This commit changes how a hardware wallet's screen-layout code handles the case where a user-interface layout has been closed or dropped earlier than expected. Previously the code would panic (crash). Now it returns a controlled RuntimeError instead. It is a defensive hardening change, not a fix for an active exploit, and it follows an earlier related pull request.
Treat as routine hardening; include in normal review and testing. Verify that callers of the updated methods handle the new RuntimeError gracefully and that no code path still assumes root() is infallible. No urgent security response is indicated by the diff alone.
Security signals we found
Replaces unwrap/panic with explicit error handling for dropped layout state
Adds new RuntimeError constant 'No layout' for graceful failure
Follow-up to prior PR #7282, suggesting ongoing hardening of layout lifecycle
No changelog entry, consistent with internal refactor/hardening
Evidence from the diff
In core/embed/rust/src/ui/layout/obj.rs, helper methods root() and root_mut() are changed from unwrap! (which panics on None) to returning Result with a new NO_LAYOUT RuntimeError. Callers are updated with ? propagation. obj_request_repaint and obj_trace now return Result<(), Error>. The C-facing extern functions already wrap the Rust block in util::try_or_raise, so the RuntimeError will be translated into a Python-level exception rather than a device panic. This is a follow-up refactor after PR #7282 to make layout teardown safer.
Changed components
core/embed/rust/src/ui/layout/obj.rsLayoutObjInner Rust struct and its root/root_mut accessorsC FFI entry points: ui_layout_paint, ui_layout_request_complete_repaint, ui_layout_traceInspect captured patch +20 / −17
diff --git a/core/embed/rust/src/ui/layout/obj.rs b/core/embed/rust/src/ui/layout/obj.rs
index 248348fb..a1ff12c2 100644
--- a/core/embed/rust/src/ui/layout/obj.rs
+++ b/core/embed/rust/src/ui/layout/obj.rs
@@ -212,6 +212,8 @@ struct LayoutObjInner {
button_request: Option<ButtonRequest>,
}
+const NO_LAYOUT: Error = Error::RuntimeError(c"No layout");
+
impl LayoutObjInner {
/// Create a new `LayoutObj`, wrapping a root component.
#[inline(never)]
@@ -229,7 +231,7 @@ impl LayoutObjInner {
};
// invoke the initial placement
- new.root_mut().place();
+ new.root_mut()?.place();
// cause a repaint pass to update the number of pages
let msg = new.obj_event(Event::RequestPaint);
assert!(matches!(msg, Ok(s) if s == Obj::const_none()));
@@ -247,22 +249,23 @@ impl LayoutObjInner {
self.timer_fn = timer_fn;
}
- fn root(&self) -> &impl Deref<Target = dyn LayoutMaybeTrace> {
- unwrap!(self.root.as_ref())
+ fn root(&self) -> Result<&impl Deref<Target = dyn LayoutMaybeTrace>, Error> {
+ self.root.as_ref().ok_or(NO_LAYOUT)
}
- fn root_mut(&mut self) -> &mut impl DerefMut<Target = dyn LayoutMaybeTrace> {
- unwrap!(self.root.as_mut())
+ fn root_mut(&mut self) -> Result<&mut impl DerefMut<Target = dyn LayoutMaybeTrace>, Error> {
+ self.root.as_mut().ok_or(NO_LAYOUT)
}
- fn obj_request_repaint(&mut self) {
+ fn obj_request_repaint(&mut self) -> Result<(), Error> {
self.repaint = Repaint::Full;
let mut event_ctx = EventCtx::new();
- let paint_msg = self.root_mut().event(&mut event_ctx, Event::RequestPaint);
+ let paint_msg = self.root_mut()?.event(&mut event_ctx, Event::RequestPaint);
// paint_msg must not change the state
assert!(paint_msg.is_none());
// there must be no timers set
assert!(event_ctx.pop_timer().is_none());
+ Ok(())
}
/// Run an event pass over the component tree. After the traversal, any
@@ -270,7 +273,7 @@ impl LayoutObjInner {
/// in case the timer callback raises or one of the components returns
/// an error, `Ok` with the message otherwise.
fn obj_event(&mut self, event: Event) -> Result<Obj, Error> {
- let root = unwrap!(self.root.as_mut());
+ let root = self.root.as_mut().ok_or(NO_LAYOUT)?;
// Get the event context ready for a new event
self.event_ctx.clear();
@@ -295,7 +298,7 @@ impl LayoutObjInner {
// Check if we should repaint next time
if self.event_ctx.needs_repaint_root() {
- self.obj_request_repaint();
+ self.obj_request_repaint()?;
} else if self.event_ctx.needs_repaint() && self.repaint == Repaint::None {
self.repaint = Repaint::Partial;
}
@@ -329,7 +332,7 @@ impl LayoutObjInner {
if self.repaint != Repaint::None {
self.repaint = Repaint::None;
- self.root_mut().paint().map(|_| true)
+ self.root_mut()?.paint().map(|_| true)
} else {
Ok(false)
}
@@ -339,7 +342,7 @@ impl LayoutObjInner {
/// with each piece of tracing information. Panics in case the callback
/// raises an exception.
#[cfg(feature = "ui_debug")]
- fn obj_trace(&self, callback: Obj) {
+ fn obj_trace(&self, callback: Obj) -> Result<(), Error> {
use crate::trace::JsonTracer;
let mut tracer = JsonTracer::new(|text: &str| {
@@ -354,9 +357,9 @@ impl LayoutObjInner {
// to claim that it implements `Trace`, and we also can't upcast it to
// `&dyn Trace` because trait upcasting is unstable.
// Luckily, calling `root.trace()` works perfectly fine in spite of the above.)
- tracer.root(&|t| {
- self.root().trace(t);
- });
+ let root = self.root()?;
+ tracer.root(&|t| root.trace(t));
+ Ok(())
}
fn obj_page_count(&self) -> Obj {
@@ -375,7 +378,7 @@ impl LayoutObjInner {
}
fn obj_return_value(&self) -> Result<Obj, Error> {
- self.root()
+ self.root()?
.value()
.cloned()
.unwrap_or(Ok(Obj::const_none()))
@@ -651,7 +654,7 @@ extern "C" fn ui_layout_paint(this: Obj) -> Obj {
extern "C" fn ui_layout_request_complete_repaint(this: Obj) -> Obj {
let block = || {
let this: Gc<LayoutObj> = this.try_into()?;
- this.inner_mut().obj_request_repaint();
+ this.inner_mut().obj_request_repaint()?;
Ok(Obj::const_none())
};
unsafe { util::try_or_raise(block) }
@@ -703,7 +706,7 @@ pub extern "C" fn ui_debug_layout_type() -> &'static Type {
extern "C" fn ui_layout_trace(this: Obj, callback: Obj) -> Obj {
let block = || {
let this: Gc<LayoutObj> = this.try_into()?;
- this.inner_mut().obj_trace(callback);
+ this.inner_mut().obj_trace(callback)?;
Ok(Obj::const_none())
};
unsafe { util::try_or_raise(block) }
Why this scored 20/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.