From 55917801a530b74c2c654ce8506fe5d05322dd94 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Wed, 20 Dec 2023 16:10:54 +0100 Subject: [PATCH 01/14] feat: add some initial cargo-fuzz test for queues --- Cargo.toml | 2 + fuzz.sh | 7 +++ fuzz/Cargo.toml | 26 ++++++++++++ fuzz/fuzz_targets/queue.rs | 87 ++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 +- src/mock_flash.rs | 1 + 6 files changed, 125 insertions(+), 2 deletions(-) create mode 100755 fuzz.sh create mode 100644 fuzz/Cargo.toml create mode 100644 fuzz/fuzz_targets/queue.rs diff --git a/Cargo.toml b/Cargo.toml index ea7173f..e75e493 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,9 +14,11 @@ keywords = ["no_std", "embedded", "flash", "storage"] [dependencies] embedded-storage = "0.3.0" defmt = { version = "0.3", optional = true } +rand = { version = "0.8.5", optional = true } [dev-dependencies] rand = "0.8.5" [features] defmt = ["dep:defmt"] +_test = ["rand"] diff --git a/fuzz.sh b/fuzz.sh new file mode 100755 index 0000000..f4fbec3 --- /dev/null +++ b/fuzz.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +set -euxo pipefail + +CPUS=32 + +cargo fuzz run --sanitizer none -j$CPUS queue diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..b63f264 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "sequential-storage-fuzz" +version = "0.0.0" +publish = false +edition = "2021" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" +sequential-storage = { path = "..", features = ["_test"] } +arbitrary = { version = "1.2.2", features = ["derive"] } + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[profile.release] +debug = 1 + +[[bin]] +name = "queue" +path = "fuzz_targets/queue.rs" +test = false +doc = false diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs new file mode 100644 index 0000000..430958f --- /dev/null +++ b/fuzz/fuzz_targets/queue.rs @@ -0,0 +1,87 @@ +#![no_main] + +use libfuzzer_sys::arbitrary::Arbitrary; +use libfuzzer_sys::fuzz_target; +use sequential_storage::mock_flash::MockFlashBase; +const MAX_VALUE_SIZE: usize = 255; + +fuzz_target!(|data: Input| fuzz(data)); + +#[derive(Arbitrary, Debug)] +struct Input { + ops: Vec, +} + +#[derive(Arbitrary, Debug)] +enum Op { + Push(PushOp), + PopMany(u8), + Pop, +} + +#[derive(Arbitrary, Debug)] +struct PushOp { + key: u16, + value_len: u8, +} + +fn fuzz(ops: Input) { + let mut flash = MockFlashBase::<4, 4, 131072>::default(); + let flash_range = 0x000..0x200000; + + let mut order = Vec::new(); + let mut buf = [0; MAX_VALUE_SIZE + 64]; + + for (i, op) in ops.ops.into_iter().enumerate() { + println!( + "==================================================== op: {:?}", + op + ); + match op { + Op::Push(mut op) => { + op.value_len %= MAX_VALUE_SIZE as u8; + + let key = op.key.to_be_bytes(); + let mut val = vec![0; op.value_len as usize]; + let val_num = i.to_be_bytes(); + let n = val_num.len().min(val.len()); + val[..n].copy_from_slice(&val_num[..n]); + + sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false) + .unwrap(); + order.push(key.to_vec()); + } + Op::Pop => { + if let Some(_key) = order.pop() { + assert!(sequential_storage::queue::pop( + &mut flash, + flash_range.clone(), + &mut buf + ) + .unwrap() + .is_some()); + } else { + assert!(sequential_storage::queue::pop( + &mut flash, + flash_range.clone(), + &mut buf + ) + .unwrap() + .is_none()); + } + } + + Op::PopMany(n) => { + let mut popper = + sequential_storage::queue::pop_many(&mut flash, flash_range.clone()); + for _i in 0..n { + if let Some(_key) = order.pop() { + assert!(popper.next(&mut buf).unwrap().is_some()); + } else { + assert!(popper.next(&mut buf).unwrap().is_none()); + } + } + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index dc0544d..6bd1eed 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![cfg_attr(not(any(test, doctest)), no_std)] +#![cfg_attr(not(any(test, doctest, feature = "_test")), no_std)] #![deny(missing_docs)] #![doc = include_str!("../README.md")] @@ -17,7 +17,7 @@ mod item; pub mod map; pub mod queue; -#[cfg(any(test, doctest))] +#[cfg(any(test, doctest, feature = "_test"))] pub mod mock_flash; /// The biggest wordsize we support. diff --git a/src/mock_flash.rs b/src/mock_flash.rs index 189d157..5e5c4c3 100644 --- a/src/mock_flash.rs +++ b/src/mock_flash.rs @@ -1,3 +1,4 @@ +#![allow(missing_docs)] use core::ops::Range; use embedded_storage::nor_flash::{ ErrorType, MultiwriteNorFlash, NorFlash, NorFlashError, NorFlashErrorKind, ReadNorFlash, From 0bc75067c885a4d8c1f06c087e114102e65e9eeb Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 11:39:01 +0100 Subject: [PATCH 02/14] fix: check result of pop operation to be the expected * Simplify value generation, no key needed * Check expected value to pop against what embedded-storage gives us * Keep track of capacity (still not working) --- fuzz/Cargo.toml | 2 ++ fuzz/fuzz_targets/queue.rs | 68 +++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index b63f264..6ce8b6f 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -11,6 +11,8 @@ cargo-fuzz = true libfuzzer-sys = "0.4" sequential-storage = { path = "..", features = ["_test"] } arbitrary = { version = "1.2.2", features = ["derive"] } +rand = "0.8.5" +embedded-storage = "0.3.0" # Prevent this from interfering with workspaces [workspace] diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs index 430958f..c8dbbb4 100644 --- a/fuzz/fuzz_targets/queue.rs +++ b/fuzz/fuzz_targets/queue.rs @@ -1,9 +1,11 @@ #![no_main] +use embedded_storage::nor_flash::ReadNorFlash; use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use sequential_storage::mock_flash::MockFlashBase; -const MAX_VALUE_SIZE: usize = 255; +use std::collections::VecDeque; +const MAX_VALUE_SIZE: usize = u8::MAX as usize; fuzz_target!(|data: Input| fuzz(data)); @@ -21,45 +23,47 @@ enum Op { #[derive(Arbitrary, Debug)] struct PushOp { - key: u16, value_len: u8, } fn fuzz(ops: Input) { - let mut flash = MockFlashBase::<4, 4, 131072>::default(); - let flash_range = 0x000..0x200000; + let mut flash = MockFlashBase::<4, 4, 256>::default(); + let flash_range = 0x000..0x1000; - let mut order = Vec::new(); - let mut buf = [0; MAX_VALUE_SIZE + 64]; + let mut order = VecDeque::new(); + let mut buf = [0; MAX_VALUE_SIZE + 1]; - for (i, op) in ops.ops.into_iter().enumerate() { + let mut used = 0; + let capacity = (0.75 * flash.capacity() as f32) as usize; + for op in ops.ops.into_iter() { println!( - "==================================================== op: {:?}", - op + "==================================================== op: {:?} (used {}/{})", + op, used, capacity, ); match op { - Op::Push(mut op) => { - op.value_len %= MAX_VALUE_SIZE as u8; + Op::Push(op) => { + let val: Vec = (0..op.value_len as usize) + .map(|_| rand::random::()) + .collect(); - let key = op.key.to_be_bytes(); - let mut val = vec![0; op.value_len as usize]; - let val_num = i.to_be_bytes(); - let n = val_num.len().min(val.len()); - val[..n].copy_from_slice(&val_num[..n]); - - sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false) - .unwrap(); - order.push(key.to_vec()); + let result = + sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false); + println!("Item size: {}. Used: {}/{}", val.len(), used, capacity); + if used + val.len() > capacity { + assert!(result.is_err()); + } else { + result.unwrap(); + used += val.len(); + order.push_back(val.to_vec()); + } } Op::Pop => { - if let Some(_key) = order.pop() { - assert!(sequential_storage::queue::pop( - &mut flash, - flash_range.clone(), - &mut buf - ) - .unwrap() - .is_some()); + if let Some(expected) = order.pop_front() { + let result = + sequential_storage::queue::pop(&mut flash, flash_range.clone(), &mut buf); + assert!(result.is_ok()); + assert_eq!(result.unwrap().unwrap(), expected); + used -= expected.len(); } else { assert!(sequential_storage::queue::pop( &mut flash, @@ -70,13 +74,15 @@ fn fuzz(ops: Input) { .is_none()); } } - Op::PopMany(n) => { let mut popper = sequential_storage::queue::pop_many(&mut flash, flash_range.clone()); for _i in 0..n { - if let Some(_key) = order.pop() { - assert!(popper.next(&mut buf).unwrap().is_some()); + if let Some(expected) = order.pop_front() { + let result = popper.next(&mut buf); + assert!(result.is_ok()); + assert_eq!(result.unwrap().unwrap(), expected); + used -= expected.len(); } else { assert!(popper.next(&mut buf).unwrap().is_none()); } From 99e6e994d589201d041c7f2d93e93eb0069ad5b5 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:34:19 +0100 Subject: [PATCH 03/14] docs: clarify that max item size is limited by page size too --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2043f9a..61944e1 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ An item is considered erased when its data CRC field is 0. *NOTE: This means the data itself is still stored on the flash when it's considered erased.* *Depending on your usecase, this might not be secure* -The length is a u16, so any item cannot be longer than 0xFFFF. +The length is a u16, so any item cannot be longer than 0xFFFF or `page size - the item header (aligned to word boundary) - page state (2 words)`. ### Inner workings for map From e2ab3bd9b1adaf7404eaf692c0946ad0be646950 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:36:20 +0100 Subject: [PATCH 04/14] add gitignore --- fuzz/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 fuzz/.gitignore diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..1a45eee --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,4 @@ +target +corpus +artifacts +coverage From a5217501bf654f06e7303b1ea806e509f703a7b4 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:37:16 +0100 Subject: [PATCH 05/14] fix: fix off-by-one found by fuzzer --- src/item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/item.rs b/src/item.rs index 9834ff9..9a699b9 100644 --- a/src/item.rs +++ b/src/item.rs @@ -320,7 +320,7 @@ pub fn find_next_free_item_spot( Ok(None) => { if ItemHeader::data_address::(current_address) + round_up_to_alignment::(data_length) - >= end_address + > end_address { // Items does not fit anymore between the current address and the end address return Ok(None); From 3d857bc8067640473a0fb62579e6a42bc0e54e2f Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:37:34 +0100 Subject: [PATCH 06/14] update gitignore --- fuzz.sh | 2 +- fuzz/.gitignore | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/fuzz.sh b/fuzz.sh index f4fbec3..9d1d058 100755 --- a/fuzz.sh +++ b/fuzz.sh @@ -4,4 +4,4 @@ set -euxo pipefail CPUS=32 -cargo fuzz run --sanitizer none -j$CPUS queue +cargo fuzz run --sanitizer none -j$CPUS queue diff --git a/fuzz/.gitignore b/fuzz/.gitignore index 1a45eee..5c404b9 100644 --- a/fuzz/.gitignore +++ b/fuzz/.gitignore @@ -2,3 +2,4 @@ target corpus artifacts coverage +Cargo.lock From 2754aa3dcde0e48e029097484d52ec58eaf33c42 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:37:53 +0100 Subject: [PATCH 07/14] fix: update fuzzer to track available space * Add convenience function for checking the largest slot available in the queue --- fuzz/fuzz_targets/queue.rs | 31 ++++++++------ src/queue.rs | 83 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 13 deletions(-) diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs index c8dbbb4..f0835bf 100644 --- a/fuzz/fuzz_targets/queue.rs +++ b/fuzz/fuzz_targets/queue.rs @@ -1,6 +1,5 @@ #![no_main] -use embedded_storage::nor_flash::ReadNorFlash; use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use sequential_storage::mock_flash::MockFlashBase; @@ -23,22 +22,25 @@ enum Op { #[derive(Arbitrary, Debug)] struct PushOp { + #[arbitrary(with = |u: &mut arbitrary::Unstructured| u.int_in_range(1..=255))] value_len: u8, } fn fuzz(ops: Input) { - let mut flash = MockFlashBase::<4, 4, 256>::default(); + const PAGES: usize = 4; + const WORD_SIZE: usize = 4; + const WORDS_PER_PAGE: usize = 256; + + let mut flash = MockFlashBase::::default(); let flash_range = 0x000..0x1000; let mut order = VecDeque::new(); let mut buf = [0; MAX_VALUE_SIZE + 1]; - let mut used = 0; - let capacity = (0.75 * flash.capacity() as f32) as usize; for op in ops.ops.into_iter() { println!( - "==================================================== op: {:?} (used {}/{})", - op, used, capacity, + "==================================================== op: {:?}", + op, ); match op { Op::Push(op) => { @@ -46,15 +48,20 @@ fn fuzz(ops: Input) { .map(|_| rand::random::()) .collect(); + let max_fit = + sequential_storage::queue::find_max_fit(&mut flash, flash_range.clone()) + .unwrap(); + let result = sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false); - println!("Item size: {}. Used: {}/{}", val.len(), used, capacity); - if used + val.len() > capacity { - assert!(result.is_err()); - } else { + + println!("Value length: {}, max fit: {}", val.len(), max_fit); + + if val.len() <= max_fit { result.unwrap(); - used += val.len(); order.push_back(val.to_vec()); + } else { + assert!(result.is_err()); } } Op::Pop => { @@ -63,7 +70,6 @@ fn fuzz(ops: Input) { sequential_storage::queue::pop(&mut flash, flash_range.clone(), &mut buf); assert!(result.is_ok()); assert_eq!(result.unwrap().unwrap(), expected); - used -= expected.len(); } else { assert!(sequential_storage::queue::pop( &mut flash, @@ -82,7 +88,6 @@ fn fuzz(ops: Input) { let result = popper.next(&mut buf); assert!(result.is_ok()); assert_eq!(result.unwrap().unwrap(), expected); - used -= expected.len(); } else { assert!(popper.next(&mut buf).unwrap().is_none()); } diff --git a/src/queue.rs b/src/queue.rs index 3ca2697..d407e2f 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -403,6 +403,89 @@ impl<'d, S: MultiwriteNorFlash> QueueIterator<'d, S> { } } +/// Find the largest size of data that can be stored. +/// +/// This will read through the entire flash to find the largest chunk of +/// data that can be stored, taking alignment requirements of the item into account. +pub fn find_max_fit( + flash: &mut S, + flash_range: Range, +) -> Result> { + assert_eq!(flash_range.start % S::ERASE_SIZE as u32, 0); + assert_eq!(flash_range.end % S::ERASE_SIZE as u32, 0); + + assert!(S::ERASE_SIZE >= S::WORD_SIZE * 4); + assert!(S::WORD_SIZE <= MAX_WORD_SIZE); + + let current_page = find_youngest_page(flash, flash_range.clone())?; + + // Find the max space available on each page until we wrap around. + let mut max_free: usize = 0; + let page_data_start_address = + calculate_page_address::(flash_range.clone(), current_page) + S::WORD_SIZE as u32; + let page_data_end_address = + calculate_page_end_address::(flash_range.clone(), current_page) - S::WORD_SIZE as u32; + + let mut current_address = page_data_start_address; + let end_address = page_data_end_address; + + while current_address < end_address { + let result = ItemHeader::read_new(flash, current_address, end_address)?; + match result { + Some(header) => current_address = header.next_item_address::(current_address), + None => { + let data_address = + round_up_to_alignment_usize::(current_address as usize + ItemHeader::LENGTH); + if data_address < end_address as usize { + let free = round_down_to_alignment_usize::( + end_address as usize - data_address as usize, + ); + if free > max_free { + max_free = free; + } + } + + break; + } + } + } + + // Check if we have space on the next page + let next_page = next_page::(flash_range.clone(), current_page); + match get_page_state(flash, flash_range.clone(), next_page)? { + PageState::Closed => { + let next_page_data_start_address = + calculate_page_address::(flash_range.clone(), next_page) + S::WORD_SIZE as u32; + let next_page_data_end_address = + calculate_page_end_address::(flash_range.clone(), next_page) + - S::WORD_SIZE as u32; + + let next_page_empty = read_item_headers( + flash, + next_page_data_start_address, + next_page_data_end_address, + |_, header, _| match header.crc { + Some(_) => ControlFlow::Break(()), + None => ControlFlow::Continue(()), + }, + )? + .is_none(); + if next_page_empty { + max_free = S::ERASE_SIZE - (2 * S::WORD_SIZE); + } + } + PageState::Open => { + max_free = S::ERASE_SIZE - (2 * S::WORD_SIZE); + } + PageState::PartialOpen => { + // This should never happen + return Err(Error::Corrupted); + } + }; + + Ok(max_free) +} + fn find_youngest_page( flash: &mut S, flash_range: Range, From 860d0f4c92aa3ce57ea983e619875f93496823cc Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 16:41:57 +0100 Subject: [PATCH 08/14] fix: add peek operations to fuzzer --- fuzz/fuzz_targets/queue.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs index f0835bf..8a1fa0f 100644 --- a/fuzz/fuzz_targets/queue.rs +++ b/fuzz/fuzz_targets/queue.rs @@ -17,6 +17,8 @@ struct Input { enum Op { Push(PushOp), PopMany(u8), + PeekMany(u8), + Peek, Pop, } @@ -93,6 +95,36 @@ fn fuzz(ops: Input) { } } } + + Op::Peek => { + if let Some(expected) = order.get(0) { + let result = + sequential_storage::queue::peek(&mut flash, flash_range.clone(), &mut buf); + assert!(result.is_ok()); + assert_eq!(result.unwrap().unwrap(), expected); + } else { + assert!(sequential_storage::queue::peek( + &mut flash, + flash_range.clone(), + &mut buf + ) + .unwrap() + .is_none()); + } + } + Op::PeekMany(n) => { + let mut peeker = + sequential_storage::queue::peek_many(&mut flash, flash_range.clone()); + for i in 0..n { + if let Some(expected) = order.get(i as usize) { + let result = peeker.next(&mut buf); + assert!(result.is_ok()); + assert_eq!(result.unwrap().unwrap(), expected); + } else { + assert!(peeker.next(&mut buf).unwrap().is_none()); + } + } + } } } } From f5da321ee003c22565de904e97556ac8534aee14 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 17:00:46 +0100 Subject: [PATCH 09/14] fix: code for verifying zero push behavior --- fuzz/fuzz_targets/queue.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs index 8a1fa0f..6580500 100644 --- a/fuzz/fuzz_targets/queue.rs +++ b/fuzz/fuzz_targets/queue.rs @@ -2,7 +2,7 @@ use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; -use sequential_storage::mock_flash::MockFlashBase; +use sequential_storage::mock_flash::{MockFlashBase, MockFlashError}; use std::collections::VecDeque; const MAX_VALUE_SIZE: usize = u8::MAX as usize; @@ -24,6 +24,7 @@ enum Op { #[derive(Arbitrary, Debug)] struct PushOp { + // TODO: Remove this once uncovering the bug #[arbitrary(with = |u: &mut arbitrary::Unstructured| u.int_in_range(1..=255))] value_len: u8, } @@ -54,9 +55,21 @@ fn fuzz(ops: Input) { sequential_storage::queue::find_max_fit(&mut flash, flash_range.clone()) .unwrap(); - let result = + let result: Result<(), sequential_storage::Error> = sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false); + // NOTE: if the queue is full, we can't push anything + // if max_fit == 0 && val.is_empty() { + // assert!(result.is_err()); + // assert!(matches!( + // result, + // Err::<(), sequential_storage::Error::>( + // sequential_storage::Error::::FullStorage + // ), + // )); + // continue; + // } + println!("Value length: {}, max fit: {}", val.len(), max_fit); if val.len() <= max_fit { From dd2ae218948afd61d9d40efeb66e4e7f18103ba3 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 18:36:30 +0100 Subject: [PATCH 10/14] fix: correctly handle 0 values in find max --- fuzz/fuzz_targets/queue.rs | 26 +++++++------------------- src/queue.rs | 16 ++++++++-------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/fuzz/fuzz_targets/queue.rs b/fuzz/fuzz_targets/queue.rs index 6580500..e186294 100644 --- a/fuzz/fuzz_targets/queue.rs +++ b/fuzz/fuzz_targets/queue.rs @@ -24,8 +24,6 @@ enum Op { #[derive(Arbitrary, Debug)] struct PushOp { - // TODO: Remove this once uncovering the bug - #[arbitrary(with = |u: &mut arbitrary::Unstructured| u.int_in_range(1..=255))] value_len: u8, } @@ -58,23 +56,13 @@ fn fuzz(ops: Input) { let result: Result<(), sequential_storage::Error> = sequential_storage::queue::push(&mut flash, flash_range.clone(), &val, false); - // NOTE: if the queue is full, we can't push anything - // if max_fit == 0 && val.is_empty() { - // assert!(result.is_err()); - // assert!(matches!( - // result, - // Err::<(), sequential_storage::Error::>( - // sequential_storage::Error::::FullStorage - // ), - // )); - // continue; - // } - - println!("Value length: {}, max fit: {}", val.len(), max_fit); - - if val.len() <= max_fit { - result.unwrap(); - order.push_back(val.to_vec()); + if let Some(max_fit) = max_fit { + if val.len() <= max_fit { + result.unwrap(); + order.push_back(val.to_vec()); + } else { + assert!(result.is_err()); + } } else { assert!(result.is_err()); } diff --git a/src/queue.rs b/src/queue.rs index d407e2f..9d7ea33 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -407,10 +407,12 @@ impl<'d, S: MultiwriteNorFlash> QueueIterator<'d, S> { /// /// This will read through the entire flash to find the largest chunk of /// data that can be stored, taking alignment requirements of the item into account. +/// +/// If there is no space left, `None` is returned. pub fn find_max_fit( flash: &mut S, flash_range: Range, -) -> Result> { +) -> Result, Error> { assert_eq!(flash_range.start % S::ERASE_SIZE as u32, 0); assert_eq!(flash_range.end % S::ERASE_SIZE as u32, 0); @@ -420,7 +422,7 @@ pub fn find_max_fit( let current_page = find_youngest_page(flash, flash_range.clone())?; // Find the max space available on each page until we wrap around. - let mut max_free: usize = 0; + let mut max_free: Option = None; let page_data_start_address = calculate_page_address::(flash_range.clone(), current_page) + S::WORD_SIZE as u32; let page_data_end_address = @@ -436,13 +438,11 @@ pub fn find_max_fit( None => { let data_address = round_up_to_alignment_usize::(current_address as usize + ItemHeader::LENGTH); - if data_address < end_address as usize { + if data_address <= end_address as usize { let free = round_down_to_alignment_usize::( end_address as usize - data_address as usize, ); - if free > max_free { - max_free = free; - } + max_free = max_free.map(|current| current.max(free)).or(Some(free)); } break; @@ -471,11 +471,11 @@ pub fn find_max_fit( )? .is_none(); if next_page_empty { - max_free = S::ERASE_SIZE - (2 * S::WORD_SIZE); + max_free.replace(S::ERASE_SIZE - (2 * S::WORD_SIZE)); } } PageState::Open => { - max_free = S::ERASE_SIZE - (2 * S::WORD_SIZE); + max_free.replace(S::ERASE_SIZE - (2 * S::WORD_SIZE)); } PageState::PartialOpen => { // This should never happen From c9c50c620c813b05f46439b8227ba087e8e4c82b Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Thu, 21 Dec 2023 19:32:43 +0100 Subject: [PATCH 11/14] Add fuzz to linked projects --- .vscode/settings.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..980099e --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,6 @@ +{ + "rust-analyzer.linkedProjects": [ + "Cargo.toml", + "fuzz/Cargo.toml" + ] +} \ No newline at end of file From ab84555976dc27164e0b201426e45d0323b61247 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 20:40:39 +0100 Subject: [PATCH 12/14] fix: add docs to mock flash public api --- src/mock_flash.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mock_flash.rs b/src/mock_flash.rs index 5e5c4c3..ffe82c6 100644 --- a/src/mock_flash.rs +++ b/src/mock_flash.rs @@ -1,9 +1,10 @@ -#![allow(missing_docs)] +//! An in-memory flash type that can be used for mocking. use core::ops::Range; use embedded_storage::nor_flash::{ ErrorType, MultiwriteNorFlash, NorFlash, NorFlashError, NorFlashErrorKind, ReadNorFlash, }; +/// State of a word in the flash. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Writable { /// Twice @@ -16,14 +17,20 @@ pub enum Writable { use Writable::*; +/// Base type for in memory flash that can be used for mocking. #[derive(Debug, Clone)] pub struct MockFlashBase { writable: Vec, data: Vec, + /// Number of erases done. pub erases: u32, + /// Number of reads done. pub reads: u32, + /// Number of writes done. pub writes: u32, + /// The chance for a bit flip to happen. pub write_bit_flip_chance: f32, + /// Check that all write locations are writeable. pub use_strict_write_count: bool, } @@ -43,6 +50,7 @@ impl const PAGE_BYTES: usize = PAGE_WORDS * BYTES_PER_WORD; + /// Create a new flash instance. pub fn new(write_bit_flip_chance: f32, use_strict_write_count: bool) -> Self { Self { writable: vec![T; Self::CAPACITY_WORDS], @@ -55,10 +63,12 @@ impl } } + /// Get a reference to the underlying data. pub fn as_bytes(&self) -> &[u8] { &self.data } + /// Get a mutable reference to the underlying data. pub fn as_bytes_mut(&mut self) -> &mut [u8] { &mut self.data } @@ -203,10 +213,14 @@ impl N } } +/// Errors reported by mock flash. #[derive(Debug, Clone, PartialEq, Eq)] pub enum MockFlashError { + /// Operation out of bounds. OutOfBounds, + /// Offset or data not aligned. NotAligned, + /// Location not writeable. NotWritable(u32), } From 7500b2f431bab81ec4183e9f7778e85743f45638 Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 21:17:05 +0100 Subject: [PATCH 13/14] fix: mock flash docs --- src/lib.rs | 1 + src/mock_flash.rs | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 6bd1eed..6659351 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ pub mod map; pub mod queue; #[cfg(any(test, doctest, feature = "_test"))] +/// An in-memory flash type that can be used for mocking. pub mod mock_flash; /// The biggest wordsize we support. diff --git a/src/mock_flash.rs b/src/mock_flash.rs index ffe82c6..84aeaba 100644 --- a/src/mock_flash.rs +++ b/src/mock_flash.rs @@ -1,4 +1,3 @@ -//! An in-memory flash type that can be used for mocking. use core::ops::Range; use embedded_storage::nor_flash::{ ErrorType, MultiwriteNorFlash, NorFlash, NorFlashError, NorFlashErrorKind, ReadNorFlash, From 37723f34cc09acab44de2f49717d230899c7e7ed Mon Sep 17 00:00:00 2001 From: Ulf Lilleengen Date: Thu, 21 Dec 2023 21:17:13 +0100 Subject: [PATCH 14/14] chore: lookup next page first --- src/queue.rs | 62 ++++++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/queue.rs b/src/queue.rs index 9d7ea33..0fcd32d 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -421,35 +421,6 @@ pub fn find_max_fit( let current_page = find_youngest_page(flash, flash_range.clone())?; - // Find the max space available on each page until we wrap around. - let mut max_free: Option = None; - let page_data_start_address = - calculate_page_address::(flash_range.clone(), current_page) + S::WORD_SIZE as u32; - let page_data_end_address = - calculate_page_end_address::(flash_range.clone(), current_page) - S::WORD_SIZE as u32; - - let mut current_address = page_data_start_address; - let end_address = page_data_end_address; - - while current_address < end_address { - let result = ItemHeader::read_new(flash, current_address, end_address)?; - match result { - Some(header) => current_address = header.next_item_address::(current_address), - None => { - let data_address = - round_up_to_alignment_usize::(current_address as usize + ItemHeader::LENGTH); - if data_address <= end_address as usize { - let free = round_down_to_alignment_usize::( - end_address as usize - data_address as usize, - ); - max_free = max_free.map(|current| current.max(free)).or(Some(free)); - } - - break; - } - } - } - // Check if we have space on the next page let next_page = next_page::(flash_range.clone(), current_page); match get_page_state(flash, flash_range.clone(), next_page)? { @@ -471,11 +442,11 @@ pub fn find_max_fit( )? .is_none(); if next_page_empty { - max_free.replace(S::ERASE_SIZE - (2 * S::WORD_SIZE)); + return Ok(Some(S::ERASE_SIZE - (2 * S::WORD_SIZE))); } } PageState::Open => { - max_free.replace(S::ERASE_SIZE - (2 * S::WORD_SIZE)); + return Ok(Some(S::ERASE_SIZE - (2 * S::WORD_SIZE))); } PageState::PartialOpen => { // This should never happen @@ -483,6 +454,35 @@ pub fn find_max_fit( } }; + // See how much space we can ind in the current page. + let mut max_free: Option = None; + let page_data_start_address = + calculate_page_address::(flash_range.clone(), current_page) + S::WORD_SIZE as u32; + let page_data_end_address = + calculate_page_end_address::(flash_range.clone(), current_page) - S::WORD_SIZE as u32; + + let mut current_address = page_data_start_address; + let end_address = page_data_end_address; + + while current_address < end_address { + let result = ItemHeader::read_new(flash, current_address, end_address)?; + match result { + Some(header) => current_address = header.next_item_address::(current_address), + None => { + let data_address = + round_up_to_alignment_usize::(current_address as usize + ItemHeader::LENGTH); + if data_address <= end_address as usize { + let free = round_down_to_alignment_usize::( + end_address as usize - data_address as usize, + ); + max_free = max_free.map(|current| current.max(free)).or(Some(free)); + } + + break; + } + } + } + Ok(max_free) }