Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esp-storage: Fix incorrect usage of MaybeUninit #1902

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions esp-storage/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fix incorrect usage of MaybeUninit (#1902)

### Removed

## 0.3.0 - 2023-08-16
Expand Down
65 changes: 65 additions & 0 deletions esp-storage/src/buffer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use core::{mem::MaybeUninit, slice};

#[cfg(feature = "nor-flash")]
pub fn uninit_slice(bytes: &[u8]) -> &[MaybeUninit<u8>] {
unsafe { core::mem::transmute(bytes) }
}

#[cfg(feature = "nor-flash")]
pub fn uninit_slice_mut(bytes: &mut [u8]) -> &mut [MaybeUninit<u8>] {
unsafe { core::mem::transmute(bytes) }
}

pub type FlashWordBuffer = FlashBuffer<4, 1>;
DBLouis marked this conversation as resolved.
Show resolved Hide resolved
pub type FlashSectorBuffer = FlashBuffer<4096, 1024>;

#[repr(C)]
pub union FlashBuffer<const N: usize, const M: usize> {
bytes: [MaybeUninit<u8>; N],
words: [MaybeUninit<u32>; M],
}

impl<const N: usize, const M: usize> FlashBuffer<N, M> {
const _CHECK: () = {
assert!(N == M * 4);
};

pub const fn uninit() -> Self {
let _ = Self::_CHECK;
Self {
words: [MaybeUninit::uninit(); M],
}
}

pub fn as_bytes(&self) -> &[MaybeUninit<u8>] {
unsafe { self.bytes.as_ref() }
}

pub fn as_bytes_mut(&mut self) -> &mut [MaybeUninit<u8>] {
unsafe { self.bytes.as_mut() }
}

pub fn as_words(&self) -> &[MaybeUninit<u32>] {
unsafe { self.words.as_ref() }
}

pub fn as_words_mut(&mut self) -> &mut [MaybeUninit<u32>] {
unsafe { self.words.as_mut() }
}

pub unsafe fn assume_init_bytes(&self) -> &[u8] {
slice::from_raw_parts(self.bytes.as_ptr() as *const u8, self.bytes.len())
}

pub unsafe fn assume_init_bytes_mut(&mut self) -> &mut [u8] {
slice::from_raw_parts_mut(self.bytes.as_mut_ptr() as *mut u8, self.bytes.len())
}

pub unsafe fn assume_init_words(&self) -> &[u32] {
slice::from_raw_parts(self.words.as_ptr() as *const u32, self.words.len())
}

pub unsafe fn assume_init_words_mut(&mut self) -> &mut [u32] {
slice::from_raw_parts_mut(self.words.as_mut_ptr() as *mut u32, self.words.len())
}
}
34 changes: 7 additions & 27 deletions esp-storage/src/common.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,6 @@
use core::ops::{Deref, DerefMut};
use core::mem::MaybeUninit;

use crate::chip_specific;

#[repr(C, align(4))]
pub struct FlashSectorBuffer {
// NOTE: Ensure that no unaligned fields are added above `data` to maintain its required
// alignment
data: [u8; FlashStorage::SECTOR_SIZE as usize],
}

impl Deref for FlashSectorBuffer {
type Target = [u8; FlashStorage::SECTOR_SIZE as usize];

fn deref(&self) -> &Self::Target {
&self.data
}
}

impl DerefMut for FlashSectorBuffer {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.data
}
}
use crate::{buffer::*, chip_specific};

#[derive(Debug)]
#[non_exhaustive]
Expand Down Expand Up @@ -71,8 +50,9 @@ impl FlashStorage {
#[cfg(any(feature = "esp32", feature = "esp32s2"))]
const ADDR: u32 = 0x1000;

let mut buffer = [0u8; 8];
storage.internal_read(ADDR, &mut buffer).ok();
let mut buffer = FlashWordBuffer::uninit();
storage.internal_read(ADDR, buffer.as_bytes_mut()).unwrap();
let buffer = unsafe { buffer.assume_init_bytes() };
let mb = match buffer[3] & 0xf0 {
0x00 => 1,
0x10 => 2,
Expand Down Expand Up @@ -115,11 +95,11 @@ impl FlashStorage {
pub(crate) fn internal_read(
&mut self,
offset: u32,
bytes: &mut [u8],
bytes: &mut [MaybeUninit<u8>],
) -> Result<(), FlashStorageError> {
check_rc(chip_specific::esp_rom_spiflash_read(
offset,
bytes.as_ptr() as *mut u32,
bytes.as_mut_ptr() as *mut u32,
bytes.len() as u32,
))
}
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ pub struct EspRomSpiflashChipT {

#[inline(never)]
#[link_section = ".rwtext"]
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| {
spiflash_wait_for_ready();
unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
}
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32c2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const ESP_ROM_SPIFLASH_UNLOCK: u32 = 0x40000140;
const ESP_ROM_SPIFLASH_ERASE_SECTOR: u32 = 0x40000130;
const ESP_ROM_SPIFLASH_WRITE: u32 = 0x40000138;

pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32c3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const ESP_ROM_SPIFLASH_UNLOCK: u32 = 0x40000140;
const ESP_ROM_SPIFLASH_ERASE_SECTOR: u32 = 0x40000128;
const ESP_ROM_SPIFLASH_WRITE: u32 = 0x4000012c;

pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32c6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const ESP_ROM_SPIFLASH_UNLOCK: u32 = 0x40000154;
const ESP_ROM_SPIFLASH_ERASE_SECTOR: u32 = 0x40000144;
const ESP_ROM_SPIFLASH_WRITE: u32 = 0x4000014c;

pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32h2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const ESP_ROM_SPIFLASH_UNLOCK: u32 = 0x40000130;
const ESP_ROM_SPIFLASH_ERASE_SECTOR: u32 = 0x40000120;
const ESP_ROM_SPIFLASH_WRITE: u32 = 0x40000128;

pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32s2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const ESP_ROM_SPIFLASH_UNLOCK: u32 = 0x40016e88;
const ESP_ROM_SPIFLASH_ERASE_SECTOR: u32 = 0x4001716c;
const ESP_ROM_SPIFLASH_WRITE: u32 = 0x400171cc;

pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
4 changes: 2 additions & 2 deletions esp-storage/src/esp32s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ const ESP_ROM_SPIFLASH_WRITE: u32 = 0x40000a14;

#[inline(always)]
#[link_section = ".rwtext"]
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *const u32, len: u32) -> i32 {
pub(crate) fn esp_rom_spiflash_read(src_addr: u32, data: *mut u32, len: u32) -> i32 {
maybe_with_critical_section(|| unsafe {
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *const u32, u32) -> i32 =
let esp_rom_spiflash_read: unsafe extern "C" fn(u32, *mut u32, u32) -> i32 =
core::mem::transmute(ESP_ROM_SPIFLASH_READ);
esp_rom_spiflash_read(src_addr, data, len)
})
Expand Down
2 changes: 1 addition & 1 deletion esp-storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod chip_specific;
mod common;

#[cfg(any(feature = "storage", feature = "nor-flash"))]
use common::FlashSectorBuffer;
mod buffer;
#[cfg(any(feature = "storage", feature = "nor-flash"))]
pub use common::{FlashStorage, FlashStorageError};

Expand Down
66 changes: 23 additions & 43 deletions esp-storage/src/nor_flash.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use core::mem::MaybeUninit;

use embedded_storage::nor_flash::{
ErrorType,
NorFlash,
Expand All @@ -8,31 +6,13 @@ use embedded_storage::nor_flash::{
ReadNorFlash,
};

use crate::{FlashSectorBuffer, FlashStorage, FlashStorageError};

#[cfg(feature = "bytewise-read")]
#[repr(C, align(4))]
struct FlashWordBuffer {
// NOTE: Ensure that no unaligned fields are added above `data` to maintain its required
// alignment
data: [u8; FlashStorage::WORD_SIZE as usize],
}

#[cfg(feature = "bytewise-read")]
impl core::ops::Deref for FlashWordBuffer {
type Target = [u8; FlashStorage::WORD_SIZE as usize];

fn deref(&self) -> &Self::Target {
&self.data
}
}

#[cfg(feature = "bytewise-read")]
impl core::ops::DerefMut for FlashWordBuffer {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.data
}
}
use crate::buffer::FlashWordBuffer;
use crate::{
buffer::{uninit_slice, uninit_slice_mut, FlashSectorBuffer},
FlashStorage,
FlashStorageError,
};

impl FlashStorage {
#[inline(always)]
Expand Down Expand Up @@ -71,13 +51,13 @@ impl ReadNorFlash for FlashStorage {
let (offset, bytes) = {
let byte_offset = (offset % Self::WORD_SIZE) as usize;
if byte_offset > 0 {
let mut word_buffer = MaybeUninit::<FlashWordBuffer>::uninit();
let word_buffer = unsafe { word_buffer.assume_init_mut() };
let mut word_buffer = FlashWordBuffer::uninit();

let offset = offset - byte_offset as u32;
let length = bytes.len().min(word_buffer.len() - byte_offset);
let length = bytes.len().min(Self::WORD_SIZE as usize - byte_offset);

self.internal_read(offset, &mut word_buffer[..])?;
self.internal_read(offset, word_buffer.as_bytes_mut())?;
let word_buffer = unsafe { word_buffer.assume_init_bytes_mut() };
bytes[..length].copy_from_slice(&word_buffer[byte_offset..][..length]);

(offset + Self::WORD_SIZE, &mut bytes[length..])
Expand All @@ -94,38 +74,37 @@ impl ReadNorFlash for FlashStorage {
{
// Chunk already is word aligned so we can read directly to it
#[cfg(not(feature = "bytewise-read"))]
self.internal_read(offset, chunk)?;
self.internal_read(offset, uninit_slice_mut(chunk))?;

#[cfg(feature = "bytewise-read")]
{
let length = chunk.len();
let byte_length = length % Self::WORD_SIZE as usize;
let length = length - byte_length;

self.internal_read(offset, &mut chunk[..length])?;
self.internal_read(offset, &mut uninit_slice_mut(chunk)[..length])?;

// Read not aligned rest of data
if byte_length > 0 {
let mut word_buffer = MaybeUninit::<FlashWordBuffer>::uninit();
let word_buffer = unsafe { word_buffer.assume_init_mut() };
let mut word_buffer = FlashWordBuffer::uninit();

self.internal_read(offset + length as u32, &mut word_buffer[..])?;
self.internal_read(offset + length as u32, word_buffer.as_bytes_mut())?;
let word_buffer = unsafe { word_buffer.assume_init_bytes_mut() };
chunk[length..].copy_from_slice(&word_buffer[..byte_length]);
}
}
}
} else {
// Bytes buffer isn't word-aligned so we might read only via aligned buffer
let mut buffer = MaybeUninit::<FlashSectorBuffer>::uninit();
let buffer = unsafe { buffer.assume_init_mut() };
let mut buffer = FlashSectorBuffer::uninit();

for (offset, chunk) in (offset..)
.step_by(Self::SECTOR_SIZE as _)
.zip(bytes.chunks_mut(Self::SECTOR_SIZE as _))
{
// Read to temporary buffer first (chunk length is aligned)
#[cfg(not(feature = "bytewise-read"))]
self.internal_read(offset, &mut buffer[..chunk.len()])?;
self.internal_read(offset, &mut buffer.as_bytes_mut()[..chunk.len()])?;

// Read to temporary buffer first (chunk length is not aligned)
#[cfg(feature = "bytewise-read")]
Expand All @@ -138,9 +117,11 @@ impl ReadNorFlash for FlashStorage {
length
};

self.internal_read(offset, &mut buffer[..length])?;
self.internal_read(offset, &mut buffer.as_bytes_mut()[..length])?;
}

let buffer = unsafe { buffer.assume_init_bytes() };

// Copy to bytes buffer
chunk.copy_from_slice(&buffer[..chunk.len()]);
}
Expand Down Expand Up @@ -173,17 +154,16 @@ impl NorFlash for FlashStorage {
}
} else {
// Bytes buffer isn't word-aligned so we might write only via aligned buffer
let mut buffer = MaybeUninit::<FlashSectorBuffer>::uninit();
let buffer = unsafe { buffer.assume_init_mut() };
let mut buffer = FlashSectorBuffer::uninit();

for (offset, chunk) in (offset..)
.step_by(Self::SECTOR_SIZE as _)
.zip(bytes.chunks(Self::SECTOR_SIZE as _))
{
// Copy to temporary buffer first
buffer[..chunk.len()].copy_from_slice(chunk);
buffer.as_bytes_mut()[..chunk.len()].copy_from_slice(uninit_slice(chunk));
// Write from temporary buffer
self.internal_write(offset, &buffer[..chunk.len()])?;
self.internal_write(offset, chunk)?;
}
}

Expand Down
Loading
Loading