aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Jorge Aparicio <jorge@japaric.io> 2017-05-08 12:53:35 -0500
committerGravatar Jorge Aparicio <jorge@japaric.io> 2017-05-08 12:53:35 -0500
commit42968fcc495be5236856a39434c1c608940a069a (patch)
treef76d3e71e8625d256decbc1599d0d7d86637959c
parent9d2d0a1447a52ae8b504ffb1d36af39b463f5339 (diff)
downloadcortex-m-42968fcc495be5236856a39434c1c608940a069a.tar.gz
cortex-m-42968fcc495be5236856a39434c1c608940a069a.tar.zst
cortex-m-42968fcc495be5236856a39434c1c608940a069a.zip
forbid sending interrupt tokens across interrupts
which would break the `ctxt::Local` abstraction by making `Mutex` `Sync` only if the protected data is `Send`. See the CHANGELOG for details. To fully fix the memory unsafety, svd2rust needs to be updated to mark interrupt tokens as `!Send`
-rw-r--r--CHANGELOG.md118
-rw-r--r--Cargo.toml2
-rw-r--r--src/interrupt.rs11
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);
diff --git a/Cargo.toml b/Cargo.toml
index 33c6023..99e656e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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"))]
() => {}
}