Skip to content

Commit

Permalink
Fix lookahead buffer size reported to littlefs2-sys
Browse files Browse the repository at this point in the history
Previously, we reported the lookahead buffer size in bytes but
littlefs2-sys expects the lookahead buffer size as a multiple of 8
bytes.  This could lead to a buffer overflow causing filesystem
corruption.  This patch fixes the reported lookahead buffer size.

Note that Storage::LOOKAHEAD_WORDS_SIZE allows users to set invalid
values (as it is measured in 4 bytes, not in 8 bytes).  Invalid values
that were previously accepted because of the wrong buffer size
calculation can now be rejected by littlefs2-sys.

This is a combination of two previous patches:
	trussed-dev#19
	#1

Fixes: trussed-dev#16
  • Loading branch information
robin-nitrokey committed Feb 7, 2023
1 parent 7b66857 commit 82ac3eb
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 15 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed
- Made `Path::from_bytes_with_nul_unchecked` `const`.

### Fixed
- Fixed the lookahead size reported to `littlefs2-sys`. Previously, the
reported size was too large by the factor of 8, potentially leading to a
buffer overflow causing filesystem corruption. Fixing this means that
`Storage::LOOKAHEADWORD_SIZE` values that are not a multiple of 2 can now
lead to an error. Fixes [#16].

[#16]: https://github.com/trussed-dev/littlefs2/issues/16

## [v0.2.2] - 2021-03-20

### Changed
Expand Down
7 changes: 3 additions & 4 deletions src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,9 @@ pub trait Storage {
/// Must be a factor of `BLOCK_SIZE`.
type CACHE_SIZE: ArrayLength<u8>;

/// littlefs itself has a `LOOKAHEAD_SIZE`, which must be a multiple of 8,
/// as it stores data in a bitmap. It also asks for 4-byte aligned buffers.
/// Hence, we further restrict `LOOKAHEAD_SIZE` to be a multiple of 32.
/// Our LOOKAHEADWORDS_SIZE is this multiple.
/// littlefs itself has a `LOOKAHEAD_SIZE`, which must be a multiple of 8 bytes. For
/// historical reasons, `LOOKAHEADWORDS_SIZE` is measured in 4 bytes. This means that users
/// must always provide a multiple of 2 here.
type LOOKAHEADWORDS_SIZE: ArrayLength<u32>;
// type LOOKAHEAD_SIZE: ArrayLength<u8>;

Expand Down
9 changes: 6 additions & 3 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ struct Cache<Storage: driver::Storage> {
read: Bytes<Storage::CACHE_SIZE>,
write: Bytes<Storage::CACHE_SIZE>,
// lookahead: aligned::Aligned<aligned::A4, Bytes<Storage::LOOKAHEAD_SIZE>>,
// lookahead buffer must be aligned to 32 bytes
lookahead: generic_array::GenericArray<u32, Storage::LOOKAHEADWORDS_SIZE>,
}

Expand All @@ -28,7 +29,6 @@ impl<S: driver::Storage> Cache<S> {
Self {
read: Default::default(),
write: Default::default(),
// lookahead: aligned::Aligned(Default::default()),
lookahead: Default::default(),
}
}
Expand Down Expand Up @@ -60,8 +60,7 @@ impl<Storage: driver::Storage> Allocation<Storage> {
let write_size: u32 = Storage::WRITE_SIZE as _;
let block_size: u32 = Storage::BLOCK_SIZE as _;
let cache_size: u32 = <Storage as driver::Storage>::CACHE_SIZE::U32;
let lookahead_size: u32 =
32 * <Storage as driver::Storage>::LOOKAHEADWORDS_SIZE::U32;
let lookahead_size: u32 = 4 * <Storage as driver::Storage>::LOOKAHEADWORDS_SIZE::U32;
let block_cycles: i32 = Storage::BLOCK_CYCLES as _;
let block_count: u32 = Storage::BLOCK_COUNT as _;

Expand Down Expand Up @@ -89,6 +88,10 @@ impl<Storage: driver::Storage> Allocation<Storage> {
debug_assert!(cache_size <= block_size);
debug_assert!(block_size % cache_size == 0);

// lookahead words size (measured in 4 bytes) must be a multiple of 2 so that the actual
// lookahead size is a multiple of 8 bytes
debug_assert!(lookahead_size % 2 == 0);

let cache = Cache::new();

let filename_max_plus_one: u32 = crate::consts::FILENAME_MAX_PLUS_ONE;
Expand Down
6 changes: 3 additions & 3 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ macro_rules! ram_storage { (
cache_size_ty=$crate::consts::U32,
block_size=128,
block_count=$bytes/128,
lookaheadwords_size_ty=$crate::consts::U1,
lookaheadwords_size_ty=$crate::consts::U2,
filename_max_plus_one_ty=$crate::consts::U256,
path_max_plus_one_ty=$crate::consts::U256,
result=LfsResult,
Expand All @@ -110,7 +110,7 @@ macro_rules! ram_storage { (
cache_size_ty=$crate::consts::U32,
block_size=128,
block_count=8,
lookaheadwords_size_ty=$crate::consts::U1,
lookaheadwords_size_ty=$crate::consts::U2,
filename_max_plus_one_ty=$crate::consts::U256,
path_max_plus_one_ty=$crate::consts::U256,
result=Result,
Expand Down Expand Up @@ -221,7 +221,7 @@ macro_rules! const_ram_storage { (
cache_size_ty=$crate::consts::U512,
block_size=512,
block_count=$bytes/512,
lookaheadwords_size_ty=$crate::consts::U1,
lookaheadwords_size_ty=$crate::consts::U2,
filename_max_plus_one_ty=$crate::consts::U256,
path_max_plus_one_ty=$crate::consts::U256,
result=LfsResult,
Expand Down
2 changes: 1 addition & 1 deletion src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ram_storage!(
cache_size_ty=consts::U32,
block_size=256,
block_count=512,
lookaheadwords_size_ty=consts::U1,
lookaheadwords_size_ty=consts::U2,
filename_max_plus_one_ty=consts::U256,
path_max_plus_one_ty=consts::U256,
result=Result,
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/constructors-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ ram_storage!(
cache_size_ty=consts::U32,
block_size=256,
block_count=512,
lookaheadwords_size_ty=consts::U1,
lookaheadwords_size_ty=consts::U2,
filename_max_plus_one_ty=consts::U256,
path_max_plus_one_ty=consts::U256,
result=Result,
Expand All @@ -31,7 +31,7 @@ ram_storage!(
cache_size_ty=consts::U700,
block_size=20*35,
block_count=32,
lookaheadwords_size_ty=consts::U1,
lookaheadwords_size_ty=consts::U2,
filename_max_plus_one_ty=consts::U256,
path_max_plus_one_ty=consts::U256,
result=Result,
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/sync-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ ram_storage!(
cache_size_ty=consts::U32,
block_size=256,
block_count=512,
lookaheadwords_size_ty=consts::U1,
lookaheadwords_size_ty=consts::U2,
filename_max_plus_one_ty=consts::U256,
path_max_plus_one_ty=consts::U256,
result=Result,
Expand All @@ -31,7 +31,7 @@ ram_storage!(
cache_size_ty=consts::U700,
block_size=20*35,
block_count=32,
lookaheadwords_size_ty=consts::U1,
lookaheadwords_size_ty=consts::U2,
filename_max_plus_one_ty=consts::U256,
path_max_plus_one_ty=consts::U256,
result=Result,
Expand Down

0 comments on commit 82ac3eb

Please sign in to comment.