Conversation
diondokter
left a comment
diondokter
left a comment
There was a problem hiding this comment.
Thanks for reporting!
I'm just not sure this is the best fix we can think of. Left some thoughts in the comments. What do you think?
| } | ||
| Err(Error::Corrupted { .. }) => { | ||
| self.encountered_corruption = true; | ||
| self.current_address += S::WORD_SIZE as u32; |
There was a problem hiding this comment.
I wonder if it would also be fixed by jumping ahead a full header instead of jumping by a word...
| // A torn header can end immediately before this erased area. | ||
| // Writing a new header there could complete a valid-looking | ||
| // phantom header that overlaps the new item. Stop using the | ||
| // page instead; callers will rotate to a clean page. | ||
| if headers.encountered_corruption { | ||
| return Ok(None); | ||
| } | ||
|
|
There was a problem hiding this comment.
Jumping to a new page seems a bit wasteful. Say someone has a noisy SPI or a bad NOR chip and so there's a 0.01% of a bit flip for each bit. Then on a 4k page there'll be 3 wrong bits and so up to 3 corrupted items. We'd waste a lot of space by skipping 2/3rd of pages on average.
That percentage is unrealistically high I think, but still
There was a problem hiding this comment.
I'm not sure about this new fuzz target.
The real solution is to have the existing fuzz targets be able to insert multiple shutoffs (instead of the fixed 1 shutoff they now have)
|
Also, did you use AI to generate this PR? |
|
I dont do non-AI contributions as well. |
2 participants
Closes #136.
Problem
After an item-header write is interrupted,
ItemHeaderIterskips the corrupt bytes one word at a time. The append scan then used the first erased byte as the next item address. A new header written at that address could complete a CRC-valid phantom header starting inside the torn header. That phantom item overlaps the real new item, so a cleanstore_itemcould returnOk(())while a fresh storage instance fetchedNone.Fix
Track whether
ItemHeaderIterencountered corruption during the append scan. If it did,find_next_free_item_spotreports that the page has no safe append location. Existing map/queue rotation then closes that page and writes on a clean page instead of placing a header directly after torn bytes.Verification
MockFlashBase: two shutdowns after seven bytes, reconstruction after each, then a clean store and another reconstruction.map-power-cuts, a state-machine fuzz target that repeatedly injects byte-exact shutdowns, destroys all cache state, reconstructsMapStorage, and checks the old-or-new durability invariant.cargo test --features arrayvec,heapless,heapless-09,alloc,postcard,shared-ram-ring: 61 passed; doctests passed.cargo clippy --features arrayvec,heapless,heapless-09,alloc,postcard,shared-ram-ring -- -D warnings: passed.cargo fuzz run --sanitizer none map-power-cuts -- -max_total_time=30: 212,258 executions, no failures.