diff --git a/rtic/CHANGELOG.md b/rtic/CHANGELOG.md index 22c5078fb1..0f6e85dd72 100644 --- a/rtic/CHANGELOG.md +++ b/rtic/CHANGELOG.md @@ -7,6 +7,12 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ## [Unreleased] +## [v2.1.1] - 2024-03-13 + +### Fixed + +- **Soundness fix:** `thumbv6` was subject to race in source mask. + ## [v2.1.0] - 2024-02-27 ### Added diff --git a/rtic/Cargo.toml b/rtic/Cargo.toml index d76f6e9cd4..2fcda6dbbd 100644 --- a/rtic/Cargo.toml +++ b/rtic/Cargo.toml @@ -22,7 +22,7 @@ name = "rtic" readme = "../README.md" repository = "https://github.com/rtic-rs/rtic" -version = "2.1.0" +version = "2.1.1" [package.metadata.docs.rs] features = ["rtic-macros/test-template"] diff --git a/rtic/ci/expected/racy-source-masking.run b/rtic/ci/expected/racy-source-masking.run new file mode 100644 index 0000000000..69ebf61c84 --- /dev/null +++ b/rtic/ci/expected/racy-source-masking.run @@ -0,0 +1,6 @@ +foo accessing shared2 resource start +pending bar +bar pended +foo accessing shared2 resource end +bar running +bar accesing shared2 resource diff --git a/rtic/examples/racy-source-masking.rs b/rtic/examples/racy-source-masking.rs new file mode 100644 index 0000000000..d871d0f379 --- /dev/null +++ b/rtic/examples/racy-source-masking.rs @@ -0,0 +1,74 @@ +//! Bug in sourcemasking test. + +#![no_main] +#![no_std] +#![deny(warnings)] +#![deny(unsafe_code)] +#![deny(missing_docs)] + +use panic_semihosting as _; + +#[rtic::app(device = lm3s6965, dispatchers = [GPIOA])] +mod app { + use cortex_m_semihosting::{debug, hprintln}; + + #[shared] + struct Shared { + shared1: u32, //shared between foo and bar, masks GPIOA and GPIOB + shared2: u32, //same + } + + #[local] + struct Local {} + + #[init] + fn init(_: init::Context) -> (Shared, Local) { + foo::spawn().unwrap(); + + ( + Shared { + shared1: 0, + shared2: 0, + }, + Local {}, + ) + } + + #[task(shared = [shared1, shared2])] + async fn foo(c: foo::Context) { + let mut shared1 = c.shared.shared1; + let mut shared2 = c.shared.shared2; + + shared2.lock(|shared2| { + hprintln!("foo accessing shared2 resource start"); + //GPIOA and GPIOB masked + *shared2 += 1; //OK access to shared 1 + shared1.lock(|shared1| { + //GPIOA and GPIOB masked again + *shared1 += 1; //so far so ggood + }); + + hprintln!("pending bar"); + rtic::pend(lm3s6965::Interrupt::GPIOB); + hprintln!("bar pended"); + + //GPIOA and GPIOB unmasked + //racy access to shared2! + *shared2 += 1; + hprintln!("foo accessing shared2 resource end"); + }); + + //GPIOA and GPIOB unmasked again + + debug::exit(debug::EXIT_SUCCESS); // Exit QEMU simulator + } + + #[task(binds = GPIOB, priority = 2, shared = [shared1, shared2])] + fn bar(mut c: bar::Context) { + hprintln!("bar running"); + c.shared.shared2.lock(|shared2| { + hprintln!("bar accesing shared2 resource"); + *shared2 += 1; // this can race with access in foo! + }); + } +} diff --git a/rtic/src/export/cortex_source_mask.rs b/rtic/src/export/cortex_source_mask.rs index 22a9201e52..a2ebe8c7e8 100644 --- a/rtic/src/export/cortex_source_mask.rs +++ b/rtic/src/export/cortex_source_mask.rs @@ -134,12 +134,13 @@ pub unsafe fn lock( } else { // safe to manipulate outside critical section let mask = compute_mask(0, ceiling, masks); + let old_mask = read_mask(mask); clear_enable_mask(mask); // execute closure under protection of raised system ceiling let r = f(&mut *ptr); - set_enable_mask(mask); + set_enable_mask(mask, old_mask); // safe to manipulate outside critical section r @@ -178,11 +179,26 @@ pub const fn compute_mask( // enables interrupts #[inline(always)] -unsafe fn set_enable_mask(mask: Mask) { +unsafe fn read_mask(mask: Mask) -> Mask { + let mut out = Mask([0; M]); + for i in 0..M { // This check should involve compile time constants and be optimized out. if mask.0[i] != 0 { - (*NVIC::PTR).iser[i].write(mask.0[i]); + out.0[i] = (*NVIC::PTR).iser[i].read(); + } + } + + out +} + +// enables interrupts +#[inline(always)] +unsafe fn set_enable_mask(mask: Mask, old_mask: Mask) { + for i in 0..M { + // This check should involve compile time constants and be optimized out. + if mask.0[i] != 0 { + (*NVIC::PTR).iser[i].write(old_mask.0[i]); } } }