diff options
| -rw-r--r-- | CHANGELOG.md | 118 | ||||
| -rw-r--r-- | Cargo.toml | 2 | ||||
| -rw-r--r-- | src/interrupt.rs | 11 |
3 files changed, 115 insertions, 16 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f4e3396..c166a9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,97 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +## [v0.2.6] - 2017-05-08 + +### Fixed + +- [breaking-change]. MEMORY UNSAFETY. `Mutex` could be used as a channel to send + interrupt tokens from one interrupt to other thus breaking the context `Local` + abstraction. See reproduction case below. This has been fixed by making + `Mutex` `Sync` only if the protected data is `Send`. + +``` rust +#![feature(const_fn)] +#![feature(used)] +#![no_std] + +extern crate cortex_m; +extern crate cortex_m_rt; +extern crate stm32f30x; + +use core::cell::RefCell; + +use cortex_m::ctxt::Local; +use cortex_m::interrupt::Mutex; +use stm32f30x::interrupt::{self, Exti0, Exti1}; + +fn main() { + // .. + + // trigger exti0 + // then trigger exti0 again +} + +static CHANNEL: Mutex<RefCell<Option<Exti0>>> = Mutex::new(RefCell::new(None)); +// Supposedly task *local* data +static LOCAL: Local<i32, Exti0> = Local::new(0); + +extern "C" fn exti0(mut ctxt: Exti0) { + static FIRST: Local<bool, Exti0> = Local::new(true); + + let first = *FIRST.borrow(&ctxt); + + // toggle + if first { + *FIRST.borrow_mut(&mut ctxt) = false; + } + + if first { + cortex_m::interrupt::free( + |cs| { + let channel = CHANNEL.borrow(cs); + + // BAD: transfer interrupt token to another interrupt + *channel.borrow_mut() = Some(ctxt); + }, + ); + + return; + } + let _local = LOCAL.borrow_mut(&mut ctxt); + + // .. + + // trigger exti1 here + + // .. + + // `LOCAL` mutably borrowed up to this point +} + +extern "C" fn exti1(_ctxt: Exti1) { + cortex_m::interrupt::free(|cs| { + let channel = CHANNEL.borrow(cs); + let mut channel = channel.borrow_mut(); + + if let Some(mut other_task) = channel.take() { + // BAD: `exti1` has access to `exti0`'s interrupt token + // so it can now mutably access local while `exti0` is also using it + let _local = LOCAL.borrow_mut(&mut other_task); + } + }); +} + +#[allow(dead_code)] +#[used] +#[link_section = ".rodata.interrupts"] +static INTERRUPTS: interrupt::Handlers = interrupt::Handlers { + Exti0: exti0, + Exti1: exti1, + ..interrupt::DEFAULT_HANDLERS +}; +``` + ## [v0.2.5] - 2017-05-07 ### Added @@ -15,17 +106,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed -- MEMORY SAFETY. `interrupt::enable` was safe to call inside an - `interrupt::free` critical section thus breaking the preemption protection. - The `interrupt::enable` method is now `unsafe`. +- [breaking-change]. MEMORY UNSAFETY. `interrupt::enable` was safe to call + inside an `interrupt::free` critical section thus breaking the preemption + protection. The `interrupt::enable` method is now `unsafe`. ## [v0.2.4] - 2017-04-20 - YANKED ### Fixed -- MEMORY SAFETY. `interrupt::free` leaked the critical section making it - possible to access a `Mutex` when interrupts are enabled (see below). This has - been fixed by changing the signature of `interrupt::free`. +- [breaking-change]. MEMORY UNSAFETY. `interrupt::free` leaked the critical + section making it possible to access a `Mutex` when interrupts are enabled + (see below). This has been fixed by changing the signature of + `interrupt::free`. ``` rust static FOO: Mutex<bool> = Mutex::new(false); @@ -41,18 +133,18 @@ fn main() { ### Fixed -- MEMORY SAFETY. Some concurrency models that use "partial" critical sections - (cf. BASEPRI) can be broken by changing the priority of interrupts or by - changing BASEPRI in some scenarios. For this reason `NVIC.set_priority` and - `register::basepri::write` are now `unsafe`. +- [breaking-change]. MEMORY UNSAFETY. Some concurrency models that use "partial" + critical sections (cf. BASEPRI) can be broken by changing the priority of + interrupts or by changing BASEPRI in some scenarios. For this reason + `NVIC.set_priority` and `register::basepri::write` are now `unsafe`. ## [v0.2.2] - 2017-04-08 - YANKED ### Fixed -- MEMORY SAFETY BUG. The `Mutex.borrow_mut` method has been removed as it can be - used to bypass Rust's borrow checker and get, for example, two mutable - references to the same data. +- [breaking-change]. MEMORY UNSAFETY. The `Mutex.borrow_mut` method has been + removed as it can be used to bypass Rust's borrow checker and get, for + example, two mutable references to the same data. ``` rust static FOO: Mutex<bool> = Mutex::new(false); @@ -6,7 +6,7 @@ keywords = ["arm", "cortex-m", "register", "peripheral"] license = "MIT OR Apache-2.0" name = "cortex-m" repository = "https://github.com/japaric/cortex-m" -version = "0.2.5" +version = "0.2.6" [dependencies] volatile-register = "0.2.0" diff --git a/src/interrupt.rs b/src/interrupt.rs index abf9348..a6abcdd 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -27,7 +27,14 @@ pub unsafe trait Nr { fn nr(&self) -> u8; } -unsafe impl<T> Sync for Mutex<T> {} +// NOTE `Mutex` can be used as a channel so, the protected data must be `Send` +// to prevent sending non-Sendable stuff (e.g. interrupt tokens) across +// different execution contexts (e.g. interrupts) +unsafe impl<T> Sync for Mutex<T> +where + T: Send, +{ +} /// Disables all interrupts #[inline(always)] @@ -61,7 +68,7 @@ pub unsafe fn enable() { : : : "volatile"); - }, + } #[cfg(not(target_arch = "arm"))] () => {} } |