ui: keep frame during async processing
What changed, and why it matters
This commit fixes a visual glitch on the BitBox02 hardware wallet screen. During some asynchronous security operations, the screen briefly showed the 'waiting' screen between two user-interface steps, causing an annoying flash. The change tells the device to keep showing the previous screen frame while it is still busy processing a USB request locally, instead of redrawing the waiting screen. It is a user-experience fix, not a cryptographic or access-control fix.
No security action required. Treat as a normal UX/quality fix and include in regular testing/QA.
Security signals we found
UI-only change; no crypto, memory-safety, or access-control modifications
New function `is_processing_request()` exposes runtime state but does not change trust boundaries
Screen hold is bypassed when a screensaver or explicit component is present, preserving existing behavior
Popped components are still cleaned up during frame hold, avoiding a potential resource leak
Evidence from the diff
The patch adds a hold_last_frame parameter to screen_process(). When true and the screen stack is empty and no screensaver is active, the function skips rendering the waiting screen and leaves the existing framebuffer in place. The Rust async-USB layer exposes is_processing_request() to indicate when a task is actively running (not merely waiting for the next host request), and the main loop passes that flag to screen_process(). Unit tests verify that stacked components and screensavers still render, popped components are still cleaned up, and the waiting screen is created only when the hold is disabled.
Changed components
src/rust/bitbox02-rust/src/async_usb.rssrc/rust/bitbox02-rust/src/main_loop.rssrc/rust/bitbox02/src/screen.rssrc/rust/bitbox02/src/ui/ui.rssrc/ui/screen_process.csrc/ui/screen_process.htest/simulator-graphical/src/main.rstest/unit-test/test_screen_process.cInspect captured patch +168 / −7
diff --git a/src/rust/bitbox02-rust/src/async_usb.rs b/src/rust/bitbox02-rust/src/async_usb.rs
index 26d58cd..b11a7e2 100644
--- a/src/rust/bitbox02-rust/src/async_usb.rs
+++ b/src/rust/bitbox02-rust/src/async_usb.rs
@@ -109,6 +109,17 @@ pub fn is_idle() -> bool {
matches!(*USB_TASK_STATE.0.borrow(), UsbTaskState::Nothing)
}
+/// Returns true while a request is actively being processed locally.
+///
+/// Waiting to send an intermediate response or receive the next request is not local processing:
+/// in those states, the waiting screen should be allowed to render.
+pub fn is_processing_request() -> bool {
+ matches!(
+ *USB_TASK_STATE.0.borrow(),
+ UsbTaskState::Running(_, WaitingForNextRequestState::Idle)
+ )
+}
+
/// Resolves the `next_request()` future. `waiting_for_next_request()` must be true when calling
/// this, otherwise this function panics.
pub fn on_next_request(usb_in: &[u8]) {
@@ -271,8 +282,10 @@ mod tests {
for _ in 0..3 {
// No task running, can't take response.
assert_eq!(Err(CopyResponseErr::NotRunning), take_response());
+ assert!(!is_processing_request());
spawn(task, &[1, 2, 3]);
+ assert!(is_processing_request());
// Can't spawn: task already running.
assert_spawn_fails();
@@ -281,6 +294,7 @@ mod tests {
assert_eq!(Err(CopyResponseErr::NotReady), take_response());
spin();
+ assert!(!is_processing_request());
// Can't spawn: result not fetched yet
assert_spawn_fails();
@@ -305,14 +319,19 @@ mod tests {
}
spawn(task, &[1, 2, 3]);
+ assert!(is_processing_request());
spin();
+ assert!(!is_processing_request());
// Intermediate response.
assert_eq!(Ok(vec![4, 5, 6, 7]), take_response());
+ assert!(!is_processing_request());
// Send follow-up request.
assert!(waiting_for_next_request());
on_next_request(&[8, 9, 10]);
+ assert!(is_processing_request());
spin();
+ assert!(!is_processing_request());
// Intermediate response.
assert_eq!(Ok(vec![11, 12]), take_response());
@@ -320,7 +339,9 @@ mod tests {
// Send follow-up request.
assert!(waiting_for_next_request());
on_next_request(&[13, 14]);
+ assert!(is_processing_request());
spin();
+ assert!(!is_processing_request());
// Final response.
assert_eq!(Ok(vec![15, 16, 17]), take_response());
@@ -407,8 +428,10 @@ mod tests {
assert!(waiting_for_next_request());
on_next_request(&[3, 4]);
+ assert!(is_processing_request());
cancel();
assert!(is_idle());
+ assert!(!is_processing_request());
spawn(second_task, &[]);
spin();
diff --git a/src/rust/bitbox02-rust/src/main_loop.rs b/src/rust/bitbox02-rust/src/main_loop.rs
index c2d7ff1..dd9a566 100644
--- a/src/rust/bitbox02-rust/src/main_loop.rs
+++ b/src/rust/bitbox02-rust/src/main_loop.rs
@@ -160,7 +160,7 @@ pub fn main_loop<H: crate::hal::Hal>(hal: &mut H) -> ! {
#[cfg(feature = "app-u2f")]
bitbox02::u2f::process();
- bitbox02::screen::process();
+ bitbox02::screen::process(crate::async_usb::is_processing_request());
/* And finally, run the high-level event processing. */
crate::async_usb::spin();
diff --git a/src/rust/bitbox02/src/screen.rs b/src/rust/bitbox02/src/screen.rs
index 48c9d92..2226769 100644
--- a/src/rust/bitbox02/src/screen.rs
+++ b/src/rust/bitbox02/src/screen.rs
@@ -36,6 +36,6 @@ pub fn splash() {
unsafe { bitbox02_sys::screen_splash() }
}
-pub fn process() {
- unsafe { bitbox02_sys::screen_process() }
+pub fn process(hold_last_frame: bool) {
+ unsafe { bitbox02_sys::screen_process(hold_last_frame) }
}
diff --git a/src/rust/bitbox02/src/ui/ui.rs b/src/rust/bitbox02/src/ui/ui.rs
index 9063e01..82bb3b8 100644
--- a/src/rust/bitbox02/src/ui/ui.rs
+++ b/src/rust/bitbox02/src/ui/ui.rs
@@ -279,7 +279,7 @@ pub async fn confirm(params: &ConfirmParams<'_>) -> ConfirmResponse {
pub fn screen_process() {
unsafe {
- bitbox02_sys::screen_process();
+ bitbox02_sys::screen_process(false);
}
}
diff --git a/src/ui/screen_process.c b/src/ui/screen_process.c
index fc79a47..6bcfcdd 100644
--- a/src/ui/screen_process.c
+++ b/src/ui/screen_process.c
@@ -84,10 +84,20 @@ static bool _screen_has_changed(const component_t* current_component)
}
#endif
-void screen_process(void)
+void screen_process(bool hold_last_frame)
{
screen_saver_process();
+ if (hold_last_frame && screen_saver_get() == NULL && ui_screen_stack_top() == NULL) {
+#ifndef TESTING
+ // The previous component may be cleaned up and its address reused for the next component.
+ // Forget it so gesture detection is reset when the next component appears.
+ (void)_screen_has_changed(NULL);
+#endif
+ ui_screen_stack_cleanup();
+ return;
+ }
+
component_t* component = screen_process_get_top_component();
_screen_draw(component);
diff --git a/src/ui/screen_process.h b/src/ui/screen_process.h
index 736562c..e2206ce 100644
--- a/src/ui/screen_process.h
+++ b/src/ui/screen_process.h
@@ -30,8 +30,11 @@ void screen_process_waiting_switch_to_lockscreen(void);
*
* This function will update the screen (if needed)
* and process gesture-related events.
+ *
+ * If `hold_last_frame` is true and there is no active screen component, the waiting screen is not
+ * rendered and the existing framebuffer remains visible.
*/
-void screen_process(void);
+void screen_process(bool hold_last_frame);
/**
* The screen is refreshed every SCREEN_FRAME_RATE event loops cycles.
diff --git a/test/simulator-graphical/src/main.rs b/test/simulator-graphical/src/main.rs
index b84bd61..bae783c 100644
--- a/test/simulator-graphical/src/main.rs
+++ b/test/simulator-graphical/src/main.rs
@@ -677,7 +677,7 @@ impl ApplicationHandler<UserEvent> for App {
}
// Business logic
bitbox02_rust::async_usb::spin();
- bitbox02::screen::process();
+ bitbox02::screen::process(bitbox02_rust::async_usb::is_processing_request());
if let Some(ref mut task) = self.startup_task {
if let Ready(_startup) = util::bb02_async::spin(task) {
diff --git a/test/unit-test/CMakeLists.txt b/test/unit-test/CMakeLists.txt
index a7ce11d..e1fa4ac 100644
--- a/test/unit-test/CMakeLists.txt
+++ b/test/unit-test/CMakeLists.txt
@@ -37,6 +37,8 @@ else()
""
random
"-Wl,--wrap=rand,--wrap=rust_sha256"
+ screen_process
+ "-Wl,--wrap=screen_saver_get,--wrap=screen_saver_process,--wrap=waiting_create"
ui_components
""
ui_util
diff --git a/test/unit-test/test_screen_process.c b/test/unit-test/test_screen_process.c
new file mode 100644
index 0000000..43b311d
--- /dev/null
+++ b/test/unit-test/test_screen_process.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include <setjmp.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <cmocka.h>
+
+#include <ui/screen_process.h>
+#include <ui/screen_stack.h>
+
+typedef struct {
+ int renders;
+ int cleanups;
+ bool free_on_cleanup;
+} component_state_t;
+
+static void _component_cleanup(component_t* component)
+{
+ component_state_t* state = component->data;
+ state->cleanups++;
+ if (state->free_on_cleanup) {
+ free(component);
+ }
+}
+
+static void _component_render(component_t* component)
+{
+ component_state_t* state = component->data;
+ state->renders++;
+}
+
+static const component_functions_t COMPONENT_FUNCTIONS = {
+ .cleanup = _component_cleanup,
+ .render = _component_render,
+ .on_event = NULL,
+};
+
+static component_t* _component_create(component_state_t* state)
+{
+ component_t* component = calloc(1, sizeof(component_t));
+ assert_non_null(component);
+ component->f = &COMPONENT_FUNCTIONS;
+ component->data = state;
+ return component;
+}
+
+static component_t* _screensaver = NULL;
+static component_state_t _waiting_state = {0};
+static component_t _waiting_component = {
+ .f = &COMPONENT_FUNCTIONS,
+ .data = &_waiting_state,
+};
+static int _waiting_creates = 0;
+
+component_t* __wrap_screen_saver_get(void)
+{
+ return _screensaver;
+}
+
+void __wrap_screen_saver_process(void) {}
+
+component_t* __wrap_waiting_create(void)
+{
+ _waiting_creates++;
+ return &_waiting_component;
+}
+
+static void test_screen_process_frame_hold(void** state)
+{
+ (void)state;
+
+ // Popped components must still be cleaned while the framebuffer is held.
+ component_state_t popped_state = {.free_on_cleanup = true};
+ component_t* popped = _component_create(&popped_state);
+ ui_screen_stack_push(popped);
+ ui_screen_stack_pop();
+ screen_process(true);
+ assert_int_equal(popped_state.cleanups, 1);
+ assert_int_equal(_waiting_creates, 0);
+
+ // An explicit component takes precedence over the framebuffer hold.
+ component_state_t stacked_state = {.free_on_cleanup = true};
+ component_t* stacked = _component_create(&stacked_state);
+ ui_screen_stack_push(stacked);
+ screen_process(true);
+ screen_process(true);
+ assert_true(stacked_state.renders > 0);
+ assert_int_equal(_waiting_creates, 0);
+ ui_screen_stack_pop();
+ screen_process(true);
+ assert_int_equal(stacked_state.cleanups, 1);
+
+ // The screensaver also takes precedence over the framebuffer hold.
+ component_state_t screensaver_state = {0};
+ component_t screensaver = {
+ .f = &COMPONENT_FUNCTIONS,
+ .data = &screensaver_state,
+ };
+ _screensaver = &screensaver;
+ screen_process(true);
+ screen_process(true);
+ assert_true(screensaver_state.renders > 0);
+ assert_int_equal(_waiting_creates, 0);
+ _screensaver = NULL;
+
+ // Without the hold, an empty stack retains the existing waiting-screen behavior.
+ screen_process(false);
+ screen_process(false);
+ assert_int_equal(_waiting_creates, 1);
+ assert_true(_waiting_state.renders > 0);
+}
+
+int main(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_screen_process_frame_hold),
+ };
+
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
Why this scored 18/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.