From b13ad7e80bed802468aac2d876d372c1bcde565c Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 7 May 2024 23:21:55 +0200 Subject: [PATCH] Fix PeripheralRef soundness issue allowing &T. Fix soundness issue introduced in a previous soundness fix https://github.com/embassy-rs/embassy/pull/2602 . PeripheralRef must not implement DerefMut itself, but the blanket impl must still require DerefMut. Otherwise you can create two instances of a driver on the same uart by using `&my_uart`. --- embassy-hal-internal/src/peripheral.rs | 15 ++++++++++++--- embassy-stm32/src/ucpd.rs | 22 +++++++++++----------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/embassy-hal-internal/src/peripheral.rs b/embassy-hal-internal/src/peripheral.rs index f03f41507..0b0f13338 100644 --- a/embassy-hal-internal/src/peripheral.rs +++ b/embassy-hal-internal/src/peripheral.rs @@ -1,5 +1,5 @@ use core::marker::PhantomData; -use core::ops::Deref; +use core::ops::{Deref, DerefMut}; /// An exclusive reference to a peripheral. /// @@ -155,7 +155,7 @@ pub trait Peripheral: Sized { } } -impl<'b, T: Deref> Peripheral for T +impl<'b, T: DerefMut> Peripheral for T where T::Target: Peripheral, { @@ -163,6 +163,15 @@ where #[inline] unsafe fn clone_unchecked(&self) -> Self::P { - self.deref().clone_unchecked() + T::Target::clone_unchecked(self) + } +} + +impl<'b, T: Peripheral> Peripheral for PeripheralRef<'_, T> { + type P = T::P; + + #[inline] + unsafe fn clone_unchecked(&self) -> Self::P { + T::clone_unchecked(self) } } diff --git a/embassy-stm32/src/ucpd.rs b/embassy-stm32/src/ucpd.rs index 63412f9b7..ff8c7aaad 100644 --- a/embassy-stm32/src/ucpd.rs +++ b/embassy-stm32/src/ucpd.rs @@ -338,7 +338,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { let dma = unsafe { Transfer::new_read( - &self.rx_dma_ch, + &mut self.rx_dma_ch, self.rx_dma_req, r.rxdr().as_ptr() as *mut u8, buf, @@ -356,7 +356,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { r.cr().modify(|w| w.set_phyrxen(true)); let _on_drop = OnDrop::new(|| { r.cr().modify(|w| w.set_phyrxen(false)); - self.enable_rx_interrupt(false); + Self::enable_rx_interrupt(false); }); poll_fn(|cx| { @@ -377,7 +377,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { Poll::Ready(ret) } else { T::state().waker.register(cx.waker()); - self.enable_rx_interrupt(true); + Self::enable_rx_interrupt(true); Poll::Pending } }) @@ -393,7 +393,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { Ok(r.rx_payszr().read().rxpaysz().into()) } - fn enable_rx_interrupt(&self, enable: bool) { + fn enable_rx_interrupt(enable: bool) { T::REGS.imr().modify(|w| w.set_rxmsgendie(enable)); } @@ -405,7 +405,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { // might still be running because there is no way to abort an ongoing // message transmission. Wait for it to finish but ignore errors. if r.cr().read().txsend() { - if let Err(TxError::HardReset) = self.wait_tx_done().await { + if let Err(TxError::HardReset) = Self::wait_tx_done().await { return Err(TxError::HardReset); } } @@ -419,7 +419,7 @@ impl<'d, T: Instance> PdPhy<'d, T> { // Start the DMA and let it do its thing in the background. let _dma = unsafe { Transfer::new_write( - &self.tx_dma_ch, + &mut self.tx_dma_ch, self.tx_dma_req, buf, r.txdr().as_ptr() as *mut u8, @@ -434,11 +434,11 @@ impl<'d, T: Instance> PdPhy<'d, T> { w.set_txsend(true); }); - self.wait_tx_done().await + Self::wait_tx_done().await } - async fn wait_tx_done(&self) -> Result<(), TxError> { - let _on_drop = OnDrop::new(|| self.enable_tx_interrupts(false)); + async fn wait_tx_done() -> Result<(), TxError> { + let _on_drop = OnDrop::new(|| Self::enable_tx_interrupts(false)); poll_fn(|cx| { let r = T::REGS; let sr = r.sr().read(); @@ -453,14 +453,14 @@ impl<'d, T: Instance> PdPhy<'d, T> { Poll::Ready(Ok(())) } else { T::state().waker.register(cx.waker()); - self.enable_tx_interrupts(true); + Self::enable_tx_interrupts(true); Poll::Pending } }) .await } - fn enable_tx_interrupts(&self, enable: bool) { + fn enable_tx_interrupts(enable: bool) { T::REGS.imr().modify(|w| { w.set_txmsgdiscie(enable); w.set_txmsgsentie(enable);