Skip to content

Commit

Permalink
Fix UB: Non-static callbacks are unsound
Browse files Browse the repository at this point in the history
  • Loading branch information
ivmarkov committed Nov 11, 2023
1 parent f917d53 commit 79fc6e7
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
9 changes: 5 additions & 4 deletions src/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1147,17 +1147,18 @@ impl<'d, T: Pin, MODE> PinDriver<'d, T, MODE> {
/// Care should be taken not to call STD, libc or FreeRTOS APIs (except for a few allowed ones)
/// in the callback passed to this function, as it is executed in an ISR context.
#[cfg(all(not(feature = "riscv-ulp-hal"), feature = "alloc"))]
pub unsafe fn subscribe(&mut self, callback: impl FnMut() + Send + 'd) -> Result<(), EspError>
pub unsafe fn subscribe<F>(&mut self, callback: F) -> Result<(), EspError>
where
F: FnMut() + Send + 'static,
MODE: InputMode,
{
extern crate alloc;

self.disable_interrupt()?;

let callback: alloc::boxed::Box<dyn FnMut() + Send + 'd> = alloc::boxed::Box::new(callback);
chip::PIN_ISR_HANDLER[self.pin.pin() as usize] =
Some(unsafe { core::mem::transmute(callback) });
let callback: alloc::boxed::Box<dyn FnMut() + Send + 'static> =
alloc::boxed::Box::new(callback);
chip::PIN_ISR_HANDLER[self.pin.pin() as usize] = Some(callback);

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions src/pcnt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,9 @@ impl<'d> PcntDriver<'d> {
/// - ()
/// - EspError
#[cfg(feature = "alloc")]
pub unsafe fn subscribe<C>(&self, callback: C) -> Result<(), EspError>
pub unsafe fn subscribe<F>(&self, callback: F) -> Result<(), EspError>
where
C: FnMut(u32) + Send + 'static,
F: FnMut(u32) + Send + 'static,
{
enable_isr_service()?;

Expand Down
9 changes: 6 additions & 3 deletions src/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,18 @@ impl<'d> TimerDriver<'d> {
/// Care should be taken not to call STD, libc or FreeRTOS APIs (except for a few allowed ones)
/// in the callback passed to this function, as it is executed in an ISR context.
#[cfg(feature = "alloc")]
pub unsafe fn subscribe(&mut self, callback: impl FnMut() + Send + 'd) -> Result<(), EspError> {
pub unsafe fn subscribe<F>(&mut self, callback: F) -> Result<(), EspError>
where
F: FnMut() + Send + 'static,
{
self.check();

self.disable_interrupt()?;

let callback: Box<dyn FnMut() + Send + 'd> = Box::new(callback);
let callback: Box<dyn FnMut() + Send + 'static> = Box::new(callback);

ISR_HANDLERS[(self.group() * timer_idx_t_TIMER_MAX + self.index()) as usize] =
Some(unsafe { core::mem::transmute(callback) });
Some(callback);

Ok(())
}
Expand Down

0 comments on commit 79fc6e7

Please sign in to comment.