diff --git a/logging/src/defmt.rs b/logging/src/defmt.rs index 3e5765d4..afd77b89 100644 --- a/logging/src/defmt.rs +++ b/logging/src/defmt.rs @@ -97,7 +97,7 @@ mod frontend { // module shows that this is always accessed within the acquire- // release critical section, so there's only one access to this // mutable data. - unsafe { &mut ENCODER } + unsafe { &mut *core::ptr::addr_of_mut!(ENCODER) } } } diff --git a/logging/src/lib.rs b/logging/src/lib.rs index 398a4849..6eadfe5d 100644 --- a/logging/src/lib.rs +++ b/logging/src/lib.rs @@ -253,7 +253,7 @@ //! [log-docs]: https://docs.rs/log/0.4/log/ #![no_std] -#![warn(missing_docs)] +#![warn(missing_docs, unsafe_op_in_unsafe_fn)] #[cfg(feature = "defmt")] pub mod defmt; diff --git a/logging/src/log/frontend.rs b/logging/src/log/frontend.rs index a2257b46..28fd95a4 100644 --- a/logging/src/log/frontend.rs +++ b/logging/src/log/frontend.rs @@ -67,11 +67,13 @@ pub(crate) unsafe fn init( // We should be preventing multiple callers with the critical section, // so the "only happens once" is to ensure that we're not changing the // static while the logger is active. - LOGGER.write(Logger { - producer: Mutex::new(RefCell::new(producer)), - filters: super::Filters(config.filters), - }); - ::log::set_logger(&*LOGGER.as_ptr()) + let logger = unsafe { + LOGGER.write(Logger { + producer: Mutex::new(RefCell::new(producer)), + filters: super::Filters(config.filters), + }) + }; + ::log::set_logger(logger) .map(|_| ::log::set_max_level(config.max_level)) .map_err(|_| crate::AlreadySetError::new(())) } diff --git a/logging/src/lpuart.rs b/logging/src/lpuart.rs index 15e79fc6..43e37810 100644 --- a/logging/src/lpuart.rs +++ b/logging/src/lpuart.rs @@ -29,8 +29,9 @@ pub(crate) const VTABLE: crate::PollerVTable = crate::PollerVTable { poll }; /// of these guarantees. The `Poller` indirectly "owns" the static mut memory, /// and the crate design ensures that there's only one `Poller` object in existence. unsafe fn poll() { - let consumer = CONSUMER.assume_init_mut(); - let channel = CHANNEL.assume_init_mut(); + // Safety: caller ensures that these are initializd, and that the function + // isn't reentrant. + let (consumer, channel) = unsafe { (CONSUMER.assume_init_mut(), CHANNEL.assume_init_mut()) }; // Could be high if the user enabled DMA interrupts. while channel.is_interrupt() { @@ -85,9 +86,14 @@ unsafe fn poll() { }; if !new.is_empty() { - channel::set_source_linear_buffer(channel, new); - channel.set_transfer_iterations(new.len().min(u16::MAX as usize) as u16); - channel.enable(); + // Safety: the buffer is static and will always be valid while the transfer + // is active. + unsafe { channel::set_source_linear_buffer(channel, new) }; + // Safety: the iterations are based on the number of elements in the collection, + // so we're not indexing out of bounds. + unsafe { channel.set_transfer_iterations(new.len().min(u16::MAX as usize) as u16) }; + // Safety: transfer is correctly set up here, and in the init method. + unsafe { channel.enable() }; } if !completed.is_empty() { @@ -111,20 +117,22 @@ pub(crate) unsafe fn init( channel.disable(); channel.clear_complete(); channel.clear_error(); - channel.set_minor_loop_bytes(core::mem::size_of::() as u32); - CONSUMER.write(consumer); - let channel = CHANNEL.write(channel); + // Safety: mutable static access. Caller only calls this once, and poll() isn't + // accessible by this point. There's no race on the consumer. + unsafe { CONSUMER.write(consumer) }; + // Safety: mutable static access. See above. + let channel = unsafe { CHANNEL.write(channel) }; channel.set_disable_on_completion(true); channel.set_interrupt_on_completion(interrupts == crate::Interrupts::Enabled); channel.set_channel_configuration(channel::Configuration::enable(lpuart.destination_signal())); - // Safety: size is appropriate for the buffer type. - channel.set_minor_loop_bytes(core::mem::size_of::() as u32); + // Safety: element size is appropriate for the buffer type. + unsafe { channel.set_minor_loop_bytes(core::mem::size_of::() as u32) }; // Safety: hardware address is valid. - channel::set_destination_hardware(channel, lpuart.destination_address()); + unsafe { channel::set_destination_hardware(channel, lpuart.destination_address()) }; lpuart.disable(|lpuart| { lpuart.disable_fifo(Direction::Tx); diff --git a/logging/src/usbd.rs b/logging/src/usbd.rs index 76da75e8..5ff98429 100644 --- a/logging/src/usbd.rs +++ b/logging/src/usbd.rs @@ -46,8 +46,22 @@ pub(crate) const VTABLE: crate::PollerVTable = crate::PollerVTable { poll }; /// and the crate design ensures that there's only one `Poller` object in existence. unsafe fn poll() { static mut CONFIGURED: bool = false; - let device = DEVICE.assume_init_mut(); - let class = CLASS.assume_init_mut(); + #[inline(always)] + fn is_configured() -> bool { + // Safety: poll() isn't reentrant. Only poll() can + // read this state. + unsafe { CONFIGURED } + } + #[inline(always)] + fn set_configured(configured: bool) { + // Safety: poll() isn't reentrant. Only poll() can + // modify this state.s + unsafe { CONFIGURED = configured }; + } + + // Safety: caller ensures that these are initializd, and that the function + // isn't reentrant. + let (device, class) = unsafe { (DEVICE.assume_init_mut(), CLASS.assume_init_mut()) }; // Is there a CDC class event, like a completed transfer? If so, check // the consumer immediately, even if a timer hasn't expired. @@ -79,18 +93,18 @@ unsafe fn poll() { let check_consumer = class_event || timer_event; if device.state() != UsbDeviceState::Configured { - if CONFIGURED { + if is_configured() { // Turn off the timer, but only if we were previously configured. device.bus().gpt_mut(GPT_INSTANCE, |gpt| gpt.stop()); } - CONFIGURED = false; + set_configured(false); // We can't use the class if we're not configured, // so bail out here. return; } // We're now configured. Are we newly configured? - if !CONFIGURED { + if !is_configured() { // Must call this when we transition into configured. device.bus().configure(); device.bus().gpt_mut(GPT_INSTANCE, |gpt| { @@ -102,7 +116,7 @@ unsafe fn poll() { gpt.run() } }); - CONFIGURED = true; + set_configured(true); } // If the host sends us data, pretend to read it. @@ -112,7 +126,9 @@ unsafe fn poll() { // There's no need to wait if we were are newly configured. if check_consumer { - let consumer = CONSUMER.assume_init_mut(); + // Safety: consumer is initialized if poll() is running. Caller ensures + // that poll() is not reentrant. + let consumer = unsafe { CONSUMER.assume_init_mut() }; if let Ok(grant) = consumer.read() { let buf = grant.buf(); // Don't try to write more than we can fit in a single packet! @@ -140,15 +156,22 @@ pub(crate) unsafe fn init( consumer: super::Consumer, config: &UsbdConfig, ) { - CONSUMER.write(consumer); + // Safety: mutable static write. This only occurs once, and poll() + // cannot run by the time we're writing this memory. + unsafe { CONSUMER.write(consumer) }; - let bus = { - let bus = imxrt_usbd::BusAdapter::without_critical_sections( - peripherals, - &ENDPOINT_MEMORY, - &ENDPOINT_STATE, - crate::config::USB_SPEED, - ); + let bus: &mut usb_device::class_prelude::UsbBusAllocator = { + // Safety: we ensure that the bus, class, and all other related USB objects + // are accessed in poll(). poll() is not reentrant, so there's no racing + // occuring across executing contexts. + let bus = unsafe { + imxrt_usbd::BusAdapter::without_critical_sections( + peripherals, + &ENDPOINT_MEMORY, + &ENDPOINT_STATE, + crate::config::USB_SPEED, + ) + }; bus.set_interrupts(interrupts == crate::Interrupts::Enabled); bus.gpt_mut(GPT_INSTANCE, |gpt| { gpt.stop(); @@ -159,12 +182,15 @@ pub(crate) unsafe fn init( gpt.reset(); }); let bus = usb_device::bus::UsbBusAllocator::new(bus); - BUS.write(bus) + // Safety: mutable static write. This only occurs once, and poll() + // cannot run by the time we're writing this memory. + unsafe { BUS.write(bus) } }; { let class = usbd_serial::CdcAcmClass::new(bus, MAX_PACKET_SIZE as u16); - CLASS.write(class); + // Safety: mutable static write. See the above discussion for BUS. + unsafe { CLASS.write(class) }; } { @@ -185,7 +211,8 @@ pub(crate) unsafe fn init( } } - DEVICE.write(device); + // Safety: mutable static write. See the above discussion for BUS. + unsafe { DEVICE.write(device) }; } } diff --git a/src/common/pit.rs b/src/common/pit.rs index f15c4245..929980ed 100644 --- a/src/common/pit.rs +++ b/src/common/pit.rs @@ -129,7 +129,9 @@ impl Pit { Const: Valid, { let register_block: &'_ crate::ral::pit::RegisterBlock = instance; - let register_block: &'static _ = core::mem::transmute(register_block); + // Safety: extending lifetime when we know that the register block has + // static lifetime. + let register_block: &'static _ = unsafe { core::mem::transmute(register_block) }; Self { instance: register_block, } diff --git a/src/lib.rs b/src/lib.rs index 48d0c389..d0993d63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,7 +81,7 @@ //! `"unproven"` feature enabled in embedded-hal 0.2. #![no_std] -#![warn(missing_docs)] +#![warn(missing_docs, unsafe_op_in_unsafe_fn)] use imxrt_ral as ral;