From 2c7bbad71a17cfcdd03a19b4d9b3e83ff442cde7 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 26 Jan 2023 22:14:48 +0100 Subject: [PATCH] Fix lookahead buffer size reported to littlefs2-sys 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: https://github.com/trussed-dev/littlefs2/pull/19 https://github.com/Nitrokey/littlefs2/pull/1 Fixes: https://github.com/trussed-dev/littlefs2/issues/16 --- CHANGELOG.md | 9 +++++++++ src/driver.rs | 7 +++---- src/fs.rs | 9 ++++++--- src/macros.rs | 6 +++--- src/tests.rs | 2 +- tests/ui/constructors-fail.rs | 4 ++-- tests/ui/sync-fail.rs | 4 ++-- 7 files changed, 26 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14c925d17..af9436c52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/driver.rs b/src/driver.rs index 3ca79fbaf..127f8d79d 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -52,10 +52,9 @@ pub trait Storage { /// Must be a factor of `BLOCK_SIZE`. type CACHE_SIZE: ArrayLength; - /// 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; // type LOOKAHEAD_SIZE: ArrayLength; diff --git a/src/fs.rs b/src/fs.rs index 7d19f8b7f..4af1865ea 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -20,6 +20,7 @@ struct Cache { read: Bytes, write: Bytes, // lookahead: aligned::Aligned>, + // lookahead buffer must be aligned to 32 bytes lookahead: generic_array::GenericArray, } @@ -28,7 +29,6 @@ impl Cache { Self { read: Default::default(), write: Default::default(), - // lookahead: aligned::Aligned(Default::default()), lookahead: Default::default(), } } @@ -60,8 +60,7 @@ impl Allocation { let write_size: u32 = Storage::WRITE_SIZE as _; let block_size: u32 = Storage::BLOCK_SIZE as _; let cache_size: u32 = ::CACHE_SIZE::U32; - let lookahead_size: u32 = - 32 * ::LOOKAHEADWORDS_SIZE::U32; + let lookahead_size: u32 = 4 * ::LOOKAHEADWORDS_SIZE::U32; let block_cycles: i32 = Storage::BLOCK_CYCLES as _; let block_count: u32 = Storage::BLOCK_COUNT as _; @@ -89,6 +88,10 @@ impl Allocation { 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; diff --git a/src/macros.rs b/src/macros.rs index 326ab20fa..fd887d6fd 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -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, @@ -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, @@ -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, diff --git a/src/tests.rs b/src/tests.rs index 35fd703f3..0bee99ff9 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -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, diff --git a/tests/ui/constructors-fail.rs b/tests/ui/constructors-fail.rs index b44b5ff36..ebc701e1c 100644 --- a/tests/ui/constructors-fail.rs +++ b/tests/ui/constructors-fail.rs @@ -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, @@ -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, diff --git a/tests/ui/sync-fail.rs b/tests/ui/sync-fail.rs index cc1a0131d..3954abf1a 100644 --- a/tests/ui/sync-fail.rs +++ b/tests/ui/sync-fail.rs @@ -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, @@ -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,