What the developer wrote
Authored by Marko Bencun
Fix timer interrupt race
The production timer future shared its state through a RefCell, but
that state is touched from two different contexts: the poll_fn closure
runs in normal Rust task context, while the timer callback can run
from the hardware timer interrupt path.
If the interrupt fired while the future was polling, both sides could
try to take a mutable RefCell borrow at the same time. RefCell only
protects against reentrant borrowing within one thread of execution;
it is not a synchronization primitive for interrupt-vs-task access.
That made the timer future racy and able to fail nondeterministically
even with only one delay future in flight.
What changed, and why it matters
This commit fixes a race condition in the BitBox02 hardware wallet's Rust-based timer code. The old code used a Rust borrow-checker helper (RefCell) to share state between normal code and a hardware timer interrupt. RefCell is not safe across interrupts, so if the timer fired while the code was checking the timer, both sides could try to modify the same data at once, leading to unpredictable failures or panics. The fix replaces the shared state with proper atomic variables and an atomic waker, which are designed for interrupt-safe concurrency.
Treat this as a reliability and potential denial-of-service hardening fix. Verify that all async timer/delay users exercise the new atomic path, and consider adding a targeted test that simulates an interrupt firing during polling. Review other Rust code that shares state with interrupt handlers for similar RefCell misuse.
Security signals we found
interrupt-vs-task race condition in timer future
use of non-thread-safe/non-interrupt-safe RefCell for shared mutable state
potential panic or nondeterministic failure of delay futures
replacement with AtomicBool and AtomicWaker for correct memory ordering
register-then-recheck pattern to close lost-wakeup window
Evidence from the diff
The patch changes src/rust/bitbox02/src/hal/timer.rs to remove a RefCell
Changed components
src/rust/bitbox02/src/hal/timer.rsBitBox02 firmware timer/delay implementationRust async runtime integration with hardware timer interruptInspect captured patch +26 / −19
diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
index 9864754..bba3734 100644
--- a/src/rust/Cargo.lock
+++ b/src/rust/Cargo.lock
@@ -260,6 +260,7 @@ dependencies = [
"bitbox-securechip-sys",
"bitbox-usb-report-queue",
"bitbox02-sys",
+ "futures-core",
"futures-lite",
"grounded",
"hex_lit",
diff --git a/src/rust/bitbox02/Cargo.toml b/src/rust/bitbox02/Cargo.toml
index 5e56d0d..6b3597b 100644
--- a/src/rust/bitbox02/Cargo.toml
+++ b/src/rust/bitbox02/Cargo.toml
@@ -21,6 +21,7 @@ util = {path = "../util"}
zeroize = { workspace = true }
bip39 = { workspace = true }
futures-lite = { workspace = true }
+futures-core = { version = "0.3.31", default-features = false }
grounded = { workspace = true }
hmac = { workspace = true }
sha2 = { workspace = true }
diff --git a/src/rust/bitbox02/src/hal/timer.rs b/src/rust/bitbox02/src/hal/timer.rs
index 5bb2d90..def9f63 100644
--- a/src/rust/bitbox02/src/hal/timer.rs
+++ b/src/rust/bitbox02/src/hal/timer.rs
@@ -15,29 +15,28 @@ impl bitbox_hal::timer::Timer for BitBox02Timer {
#[cfg(not(feature = "c-unit-testing"))]
async fn delay_for(duration: Duration) {
use alloc::boxed::Box;
- use core::cell::RefCell;
use core::ffi::c_void;
- use core::task::{Poll, Waker};
+ use core::sync::atomic::{AtomicBool, Ordering};
+ use core::task::Poll;
+ use futures_core::task::__internal::AtomicWaker;
let mut bitbox02_delay = bitbox02_sys::delay_t { id: 0 };
// Shared between the async context and the c callback
struct SharedState {
- waker: Option<Waker>,
- result: Option<()>,
+ waker: AtomicWaker,
+ done: AtomicBool,
}
- let shared_state = Box::new(RefCell::new(SharedState {
- waker: None,
- result: None,
- }));
- let shared_state_ptr = shared_state.as_ref() as *const RefCell<SharedState> as *mut c_void;
+ let shared_state = Box::new(SharedState {
+ waker: AtomicWaker::new(),
+ done: AtomicBool::new(false),
+ });
+
+ let shared_state_ptr = shared_state.as_ref() as *const SharedState as *mut c_void;
unsafe extern "C" fn callback(user_data: *mut c_void) {
- let shared_state = unsafe { &*(user_data as *mut RefCell<SharedState>) };
- let mut shared_state = shared_state.borrow_mut();
- shared_state.result = Some(());
- if let Some(waker) = shared_state.waker.as_ref() {
- waker.wake_by_ref();
- }
+ let shared_state = unsafe { &*(user_data as *mut SharedState) };
+ shared_state.done.store(true, Ordering::Release);
+ shared_state.waker.wake();
}
unsafe {
bitbox02_sys::delay_init_ms(
@@ -59,12 +58,16 @@ impl bitbox_hal::timer::Timer for BitBox02Timer {
core::future::poll_fn({
let shared_state = &shared_state;
move |cx| {
- let mut shared_state = shared_state.borrow_mut();
+ if shared_state.done.load(Ordering::Acquire) {
+ return Poll::Ready(());
+ }
- if let Some(result) = shared_state.result {
- Poll::Ready(result)
+ // Register first, then re-check the completion flag so a callback that fires
+ // between the first load and the registration cannot be missed.
+ shared_state.waker.register(cx.waker());
+ if shared_state.done.load(Ordering::Acquire) {
+ Poll::Ready(())
} else {
- shared_state.waker = Some(cx.waker().clone());
Poll::Pending
}
}
diff --git a/test/simulator-graphical-bb03/Cargo.lock b/test/simulator-graphical-bb03/Cargo.lock
index 48aebe9..61c4dd5 100644
--- a/test/simulator-graphical-bb03/Cargo.lock
+++ b/test/simulator-graphical-bb03/Cargo.lock
@@ -463,6 +463,7 @@ dependencies = [
"bitbox-securechip-sys",
"bitbox-usb-report-queue",
"bitbox02-sys",
+ "futures-core",
"futures-lite",
"grounded",
"hex_lit",
diff --git a/test/simulator-graphical/Cargo.lock b/test/simulator-graphical/Cargo.lock
index c862af9..2bf8691 100644
--- a/test/simulator-graphical/Cargo.lock
+++ b/test/simulator-graphical/Cargo.lock
@@ -407,6 +407,7 @@ dependencies = [
"bitbox-securechip-sys",
"bitbox-usb-report-queue",
"bitbox02-sys",
+ "futures-core",
"futures-lite",
"grounded",
"hex_lit",
Why this scored 49/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.