fix(soundness): disable interrupts on every RefCell flags change

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>

Author: mkroening

src/interrupt_dropper.rs

@@ -0,0 +1,50 @@
+use core::mem::ManuallyDrop;
+use core::ops::{Deref, DerefMut};
+
+/// A wrapper for dropping values while interrupts are disabled.
+pub struct InterruptDropper<T> {
+    inner: ManuallyDrop<T>,
+}
+
+impl<T> From<T> for InterruptDropper<T> {
+    #[inline]
+    fn from(value: T) -> Self {
+        Self {
+            inner: ManuallyDrop::new(value),
+        }
+    }
+}
+
+impl<T> InterruptDropper<T> {
+    #[inline]
+    pub fn into_inner(mut this: Self) -> T {
+        // SAFETY: We never use `this` after this again.
+        unsafe { ManuallyDrop::take(&mut this.inner) }
+    }
+}
+
+impl<T> Deref for InterruptDropper<T> {
+    type Target = T;
+
+    #[inline]
+    fn deref(&self) -> &Self::Target {
+        self.inner.deref()
+    }
+}
+
+impl<T> DerefMut for InterruptDropper<T> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        self.inner.deref_mut()
+    }
+}
+
+impl<T> Drop for InterruptDropper<T> {
+    #[inline]
+    fn drop(&mut self) {
+        let _guard = interrupts::disable();
+        // Drop `inner` as while we can guarentee interrupts are disabled
+        // SAFETY: This is not exposed to safe code and is not called more than once
+        unsafe { ManuallyDrop::drop(&mut self.inner) }
+    }
+}

src/lib.rs

@@ -62,6 +62,7 @@
 
 #![cfg_attr(target_os = "none", no_std)]
 
+mod interrupt_dropper;
 #[cfg(not(target_os = "none"))]
 mod local_key;
 
@@ -70,6 +71,7 @@ use core::cmp::Ordering;
 use core::ops::{Deref, DerefMut};
 use core::{fmt, mem};
 
+use self::interrupt_dropper::InterruptDropper;
 #[cfg(not(target_os = "none"))]
 pub use self::local_key::LocalKeyExt;
 
@@ -265,9 +267,10 @@ impl<T: ?Sized> InterruptRefCell<T> {
     #[cfg_attr(feature = "debug_interruptrefcell", track_caller)]
     pub fn try_borrow(&self) -> Result<InterruptRef<'_, T>, BorrowError> {
         let _guard = interrupts::disable();
-        self.inner
-            .try_borrow()
-            .map(|inner| InterruptRef { inner, _guard })
+        self.inner.try_borrow().map(|inner| {
+            let inner = InterruptDropper::from(inner);
+            InterruptRef { inner, _guard }
+        })
     }
 
     /// Mutably borrows the wrapped value.
@@ -335,9 +338,10 @@ impl<T: ?Sized> InterruptRefCell<T> {
     #[cfg_attr(feature = "debug_interruptrefcell", track_caller)]
     pub fn try_borrow_mut(&self) -> Result<InterruptRefMut<'_, T>, BorrowMutError> {
         let _guard = interrupts::disable();
-        self.inner
-            .try_borrow_mut()
-            .map(|inner| InterruptRefMut { inner, _guard })
+        self.inner.try_borrow_mut().map(|inner| {
+            let inner = InterruptDropper::from(inner);
+            InterruptRefMut { inner, _guard }
+        })
     }
 
     /// Returns a raw pointer to the underlying data in this cell.
@@ -417,6 +421,7 @@ impl<T: ?Sized> InterruptRefCell<T> {
     /// ```
     #[inline]
     pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
+        let _guard = interrupts::disable();
         self.inner.try_borrow_unguarded()
     }
 }
@@ -548,7 +553,7 @@ impl<T> From<T> for InterruptRefCell<T> {
 ///
 /// See the [module-level documentation](self) for more.
 pub struct InterruptRef<'b, T: ?Sized + 'b> {
-    inner: Ref<'b, T>,
+    inner: InterruptDropper<Ref<'b, T>>,
     _guard: interrupts::Guard,
 }
 
@@ -574,10 +579,9 @@ impl<'b, T: ?Sized> InterruptRef<'b, T> {
     #[must_use]
     #[inline]
     pub fn clone(orig: &InterruptRef<'b, T>) -> InterruptRef<'b, T> {
-        InterruptRef {
-            inner: Ref::clone(&orig.inner),
-            _guard: interrupts::disable(),
-        }
+        let _guard = interrupts::disable();
+        let inner = InterruptDropper::from(Ref::clone(&orig.inner));
+        InterruptRef { inner, _guard }
     }
 
     /// Makes a new `InterruptRef` for a component of the borrowed data.
@@ -604,10 +608,8 @@ impl<'b, T: ?Sized> InterruptRef<'b, T> {
         F: FnOnce(&T) -> &U,
     {
         let InterruptRef { inner, _guard } = orig;
-        InterruptRef {
-            inner: Ref::map(inner, f),
-            _guard,
-        }
+        let inner = InterruptDropper::from(Ref::map(InterruptDropper::into_inner(inner), f));
+        InterruptRef { inner, _guard }
     }
 
     /// Makes a new `InterruptRef` for an optional component of the borrowed data. The
@@ -640,9 +642,15 @@ impl<'b, T: ?Sized> InterruptRef<'b, T> {
         F: FnOnce(&T) -> Option<&U>,
     {
         let InterruptRef { inner, _guard } = orig;
-        match Ref::filter_map(inner, f) {
-            Ok(inner) => Ok(InterruptRef { inner, _guard }),
-            Err(inner) => Err(InterruptRef { inner, _guard }),
+        match Ref::filter_map(InterruptDropper::into_inner(inner), f) {
+            Ok(inner) => {
+                let inner = InterruptDropper::from(inner);
+                Ok(InterruptRef { inner, _guard })
+            }
+            Err(inner) => {
+                let inner = InterruptDropper::from(inner);
+                Err(InterruptRef { inner, _guard })
+            }
         }
     }
 
@@ -674,15 +682,16 @@ impl<'b, T: ?Sized> InterruptRef<'b, T> {
     where
         F: FnOnce(&T) -> (&U, &V),
     {
-        let (a, b) = Ref::map_split(orig.inner, f);
+        let _guard = interrupts::disable();
+        let (a, b) = Ref::map_split(InterruptDropper::into_inner(orig.inner), f);
         (
             InterruptRef {
-                inner: a,
-                _guard: orig._guard,
+                inner: InterruptDropper::from(a),
+                _guard,
             },
             InterruptRef {
-                inner: b,
-                _guard: interrupts::disable(),
+                inner: InterruptDropper::from(b),
+                _guard: orig._guard,
             },
         )
     }
@@ -730,10 +739,8 @@ impl<'b, T: ?Sized> InterruptRefMut<'b, T> {
         F: FnOnce(&mut T) -> &mut U,
     {
         let InterruptRefMut { inner, _guard } = orig;
-        InterruptRefMut {
-            inner: RefMut::map(inner, f),
-            _guard,
-        }
+        let inner = InterruptDropper::from(RefMut::map(InterruptDropper::into_inner(inner), f));
+        InterruptRefMut { inner, _guard }
     }
 
     /// Makes a new `InterruptRefMut` for an optional component of the borrowed data. The
@@ -774,9 +781,15 @@ impl<'b, T: ?Sized> InterruptRefMut<'b, T> {
         F: FnOnce(&mut T) -> Option<&mut U>,
     {
         let InterruptRefMut { inner, _guard } = orig;
-        match RefMut::filter_map(inner, f) {
-            Ok(inner) => Ok(InterruptRefMut { inner, _guard }),
-            Err(inner) => Err(InterruptRefMut { inner, _guard }),
+        match RefMut::filter_map(InterruptDropper::into_inner(inner), f) {
+            Ok(inner) => {
+                let inner = InterruptDropper::from(inner);
+                Ok(InterruptRefMut { inner, _guard })
+            }
+            Err(inner) => {
+                let inner = InterruptDropper::from(inner);
+                Err(InterruptRefMut { inner, _guard })
+            }
         }
     }
 
@@ -813,15 +826,16 @@ impl<'b, T: ?Sized> InterruptRefMut<'b, T> {
     where
         F: FnOnce(&mut T) -> (&mut U, &mut V),
     {
-        let (a, b) = RefMut::map_split(orig.inner, f);
+        let _guard = interrupts::disable();
+        let (a, b) = RefMut::map_split(InterruptDropper::into_inner(orig.inner), f);
         (
             InterruptRefMut {
-                inner: a,
-                _guard: orig._guard,
+                inner: InterruptDropper::from(a),
+                _guard,
             },
             InterruptRefMut {
-                inner: b,
-                _guard: interrupts::disable(),
+                inner: InterruptDropper::from(b),
+                _guard: orig._guard,
             },
         )
     }
@@ -831,7 +845,7 @@ impl<'b, T: ?Sized> InterruptRefMut<'b, T> {
 ///
 /// See the [module-level documentation](self) for more.
 pub struct InterruptRefMut<'b, T: ?Sized + 'b> {
-    inner: RefMut<'b, T>,
+    inner: InterruptDropper<RefMut<'b, T>>,
     _guard: interrupts::Guard,
 }