From 129114d45fb3e79434b62954a206549071eeabd5 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 6 Feb 2025 20:42:34 -0500 Subject: [PATCH 01/11] Simplify incompatible feature errors In the `Incompatible` enum: * Renamed `Missing` to `MissingRequiredFeatures`. * Renamed `Incompatible` to `UnsupportedFeatures`. * Removed `Unknown`, those errors are now also reported as `UnsupportedFeatures`. --- CHANGELOG.md | 4 ++++ src/error.rs | 25 ++++++++----------------- src/superblock.rs | 29 ++++++++++++++++------------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 239ca15..fc22c3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## Unreleased * Removed `Ext4Error::as_corrupt`. +* Renamed `Incompatible::Missing` to `Incompatible::MissingRequiredFeatures`. +* Renamed `Incompatible::Incompatible` to `Incompatible::UnsupportedFeatures`. +* Removed `Incompatible::Unknown`; these errors are now reported as + `Incompatible::UnsupportedFeatures`. ## 0.8.0 diff --git a/src/error.rs b/src/error.rs index 3caf193..ddd46f8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -411,22 +411,16 @@ impl From for Ext4Error { #[derive(Clone, Debug, Eq, PartialEq)] #[non_exhaustive] pub enum Incompatible { - /// One or more unknown bits are set in the incompatible feature flags. - Unknown( - /// The unknown features. - IncompatibleFeatures, - ), - - /// One or more required incompatible features are missing. - Missing( + /// One or more required features are missing. + MissingRequiredFeatures( /// The missing features. IncompatibleFeatures, ), - /// One or more disallowed incompatible features are present. + /// One or more unsupported features are present. #[allow(clippy::enum_variant_names)] - Incompatible( - /// The incompatible features. + UnsupportedFeatures( + /// The unsupported features. IncompatibleFeatures, ), @@ -472,14 +466,11 @@ pub enum Incompatible { impl Display for Incompatible { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { - Self::Unknown(feat) => { - write!(f, "unknown features: {feat:?}") - } - Self::Missing(feat) => { + Self::MissingRequiredFeatures(feat) => { write!(f, "missing required features: {feat:?}") } - Self::Incompatible(feat) => { - write!(f, "incompatible features: {feat:?}") + Self::UnsupportedFeatures(feat) => { + write!(f, "unsupported features: {feat:?}") } Self::DirectoryHash(algorithm) => { write!(f, "unsupported directory hash algorithm: {algorithm}") diff --git a/src/superblock.rs b/src/superblock.rs index 8cfcbcf..0292ee1 100644 --- a/src/superblock.rs +++ b/src/superblock.rs @@ -190,7 +190,9 @@ fn check_incompat_features( let actual_known = IncompatibleFeatures::from_bits_truncate(s_feature_incompat); if actual != actual_known { - return Err(Incompatible::Unknown(actual.difference(actual_known))); + return Err(Incompatible::UnsupportedFeatures( + actual.difference(actual_known), + )); } // TODO: for now, be strict on many incompat features. May be able to @@ -208,14 +210,14 @@ fn check_incompat_features( let present_required = actual & required_features; if present_required != required_features { - return Err(Incompatible::Missing( + return Err(Incompatible::MissingRequiredFeatures( required_features.difference(present_required), )); } let present_disallowed = actual & disallowed_features; if !present_disallowed.is_empty() { - return Err(Incompatible::Incompatible(present_disallowed)); + return Err(Incompatible::UnsupportedFeatures(present_disallowed)); } Ok(actual) @@ -344,8 +346,7 @@ mod tests { } /// Test that an error is returned if an unknown incompatible - /// feature bit is set. Test that the error value contains only the - /// unknown bits. + /// feature bit is set. #[test] fn test_unknown_incompat_flags() { let mut data = @@ -356,9 +357,9 @@ mod tests { .unwrap_err() .as_incompatible() .unwrap(), - Incompatible::Unknown(IncompatibleFeatures::from_bits_retain( - 0x2_0000 - )) + Incompatible::UnsupportedFeatures( + IncompatibleFeatures::from_bits_retain(0x2_0000) + ) ); } @@ -375,9 +376,9 @@ mod tests { // Unknown incompatible bit is an error. assert_eq!( check_incompat_features(required | 0x2_0000).unwrap_err(), - Incompatible::Unknown(IncompatibleFeatures::from_bits_retain( - 0x2_0000 - )) + Incompatible::UnsupportedFeatures( + IncompatibleFeatures::from_bits_retain(0x2_0000) + ) ); assert_eq!( @@ -386,7 +387,9 @@ mod tests { & (!IncompatibleFeatures::FILE_TYPE_IN_DIR_ENTRY.bits()) ) .unwrap_err(), - Incompatible::Missing(IncompatibleFeatures::FILE_TYPE_IN_DIR_ENTRY) + Incompatible::MissingRequiredFeatures( + IncompatibleFeatures::FILE_TYPE_IN_DIR_ENTRY + ) ); assert_eq!( @@ -394,7 +397,7 @@ mod tests { required | IncompatibleFeatures::RECOVERY.bits() ) .unwrap_err(), - Incompatible::Incompatible(IncompatibleFeatures::RECOVERY) + Incompatible::UnsupportedFeatures(IncompatibleFeatures::RECOVERY) ); } } From 406e18ccf27cd3a934a48a8076d203a99bdc468a Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Tue, 26 Nov 2024 11:44:20 -0500 Subject: [PATCH 02/11] More journal work --- CHANGELOG.md | 1 + src/checksum.rs | 5 + src/error.rs | 60 +++++++++++ src/journal.rs | 182 ++++++++++++++++++++++++++++++-- src/journal/descriptor_block.rs | 172 ++++++++++++++++++++++++++++++ src/journal/superblock.rs | 22 ++++ src/lib.rs | 2 + src/superblock.rs | 41 +++---- 8 files changed, 456 insertions(+), 29 deletions(-) create mode 100644 src/journal/descriptor_block.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index fc22c3f..e0cadfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Added `Ext4::uuid` to get the filesystem UUID. * Made the `Corrupt` type opaque. It is no longer possible to `match` on specific types of corruption. +* Added support for reading filesystems that weren't cleanly unmounted. ## 0.7.0 diff --git a/src/checksum.rs b/src/checksum.rs index 8685600..e432092 100644 --- a/src/checksum.rs +++ b/src/checksum.rs @@ -54,6 +54,11 @@ impl Checksum { self.digest.update(data); } + /// Extend the digest with a big-endian `u32`. + pub(crate) fn update_u32_be(&mut self, data: u32) { + self.update(&data.to_be_bytes()); + } + /// Extend the digest with a little-endian `u16`. pub(crate) fn update_u16_le(&mut self, data: u16) { self.update(&data.to_le_bytes()); diff --git a/src/error.rs b/src/error.rs index ddd46f8..ccf650d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -232,6 +232,30 @@ pub(crate) enum CorruptKind { /// Journal superblock checksum is invalid. JournalSuperblockChecksum, + /// Journal block size does not match the filesystem block size. + JournalBlockSize, + + /// Journal does not have the expected number of blocks. + JournalTruncated, + + /// Journal first commit doesn't match the sequence number in the superblock. + JournalSequence, + + /// Journal commit block checksum is invalid. + JournalCommitBlockChecksum, + + /// Journal descriptor block checksum is invalid. + JournalDescriptorBlockChecksum, + + /// Journal descriptor tag checksum is invalid. + JournalDescriptorTagChecksum, + + /// Not enough bytes to read a journal descriptor tag. + JournalDescriptorTagTruncated, + + /// Journal has a descriptor block that contains no tag with the last-tag flag set. + JournalDescriptorBlockMissingLastTag, + /// An inode's checksum is invalid. InodeChecksum(InodeIndex), @@ -315,6 +339,33 @@ impl Display for CorruptKind { Self::JournalSuperblockChecksum => { write!(f, "journal superblock checksum is invalid") } + Self::JournalBlockSize => { + write!( + f, + "journal block size does not match filesystem block size" + ) + } + Self::JournalTruncated => write!(f, "journal is truncated"), + Self::JournalSequence => write!( + f, + "journal's first commit doesn't match the expected sequence" + ), + Self::JournalCommitBlockChecksum => { + write!(f, "journal commit block checksum is invalid") + } + Self::JournalDescriptorBlockChecksum => { + write!(f, "journal descriptor block checksum is invalid") + } + Self::JournalDescriptorTagChecksum => { + write!(f, "journal descriptor tag checksum is invalid") + } + Self::JournalDescriptorTagTruncated => { + write!(f, "journal descriptor tag truncated") + } + Self::JournalDescriptorBlockMissingLastTag => write!( + f, + "a journal descriptor block has no tag with the last-tag flag set" + ), Self::InodeChecksum(inode) => { write!(f, "invalid checksum for inode {inode}") } @@ -461,6 +512,12 @@ pub enum Incompatible { /// The unsupported feature bits. u32, ), + + /// The journal contains an unsupported block type. + JournalBlockType( + /// Raw journal block type. + u32, + ), } impl Display for Incompatible { @@ -481,6 +538,9 @@ impl Display for Incompatible { Self::JournalSuperblockType(val) => { write!(f, "journal superblock type is not supported: {val}") } + Self::JournalBlockType(val) => { + write!(f, "journal block type is not supported: {val}") + } Self::JournalChecksumType(val) => { write!(f, "journal checksum type is not supported: {val}") } diff --git a/src/journal.rs b/src/journal.rs index 6080c8c..69a5e3c 100644 --- a/src/journal.rs +++ b/src/journal.rs @@ -6,34 +6,196 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#[expect(unused)] // TODO mod block_header; -#[expect(unused)] // TODO +mod descriptor_block; mod superblock; -use crate::{Ext4, Ext4Error}; +use crate::checksum::Checksum; +use crate::error::{CorruptKind, Ext4Error, Incompatible}; +use crate::inode::Inode; +use crate::iters::file_blocks::FileBlocks; +use crate::util::{read_u32be, usize_from_u32}; +use crate::Ext4; +use alloc::collections::BTreeMap; +use alloc::vec; +use block_header::{JournalBlockHeader, JournalBlockType}; +use descriptor_block::{ + is_descriptor_block_checksum_valid, DescriptorBlockTag, +}; +use superblock::JournalSuperblock; #[derive(Debug)] pub(crate) struct Journal { - // TODO: add journal data. + block_map: BTreeMap, } impl Journal { - /// Create an empty journal. pub(crate) fn empty() -> Self { - Self {} + Self { + block_map: BTreeMap::new(), + } } - /// Load a journal from the filesystem. + /// Load the journal. + /// + /// If the filesystem has no journal, an empty journal is returned. + /// + /// Note: ext4 is all little-endian, except for the journal, which + /// is all big-endian. pub(crate) fn load(fs: &Ext4) -> Result { - let Some(_journal_inode) = fs.0.superblock.journal_inode else { + let Some(journal_inode) = fs.0.superblock.journal_inode else { // Return an empty journal if this filesystem does not have // a journal. return Ok(Self::empty()); }; - // TODO: actually load the journal. + let journal_inode = Inode::read(fs, journal_inode)?; + let superblock = JournalSuperblock::load(fs, &journal_inode)?; - Ok(Self {}) + // Ensure the journal block size matches the rest of the + // filesystem. + let block_size = fs.0.superblock.block_size; + if superblock.block_size != block_size { + return Err(CorruptKind::JournalBlockSize.into()); + } + + let block_map = load_block_map(fs, &superblock, &journal_inode)?; + + Ok(Self { block_map }) + } + + /// Map from an absolute block index to a block in the journal. + /// + /// If the journal does not contain a replacement for the input + /// block, the input block is returned. + pub(crate) fn map_block_index(&self, block_index: u64) -> u64 { + *self.block_map.get(&block_index).unwrap_or(&block_index) + } +} + +fn load_block_map( + fs: &Ext4, + superblock: &JournalSuperblock, + journal_inode: &Inode, +) -> Result, Ext4Error> { + // Get an iterator over the journal's block indices. + let journal_block_iter = FileBlocks::new(fs.clone(), journal_inode)?; + + // Skip forward to the start block. + let mut journal_block_iter = + journal_block_iter.skip(usize_from_u32(superblock.start_block)); + + // TODO: the loop below currently returns an error if something + // bad is encountered (e.g. a wrong checksum). We should + // actually still apply valid commits, and just stop reading the + // journal when bad data is encountered. + + let mut block = vec![0; fs.0.superblock.block_size.to_usize()]; + let mut block_map = BTreeMap::new(); + let mut uncommitted_block_map = BTreeMap::new(); + let mut sequence = superblock.sequence; + while let Some(block_index) = journal_block_iter.next() { + let block_index = block_index?; + + fs.read_from_block(block_index, 0, &mut block)?; + + let Some(header) = JournalBlockHeader::read_bytes(&block)? else { + // Journal block magic is not present, so we've reached + // the end of the journal. + break; + }; + + if header.sequence != sequence { + return Err(CorruptKind::JournalSequence.into()); + } + + if header.block_type == JournalBlockType::DESCRIPTOR { + if !is_descriptor_block_checksum_valid(superblock, &block) { + return Err(CorruptKind::JournalDescriptorBlockChecksum.into()); + } + + let tags = + DescriptorBlockTag::read_bytes_to_vec(&block[12..]).unwrap(); + + for tag in &tags { + let block_index = journal_block_iter + .next() + .ok_or(CorruptKind::JournalTruncated)??; + + let mut checksum = Checksum::new(); + checksum.update(superblock.uuid.as_bytes()); + checksum.update_u32_be(sequence); + fs.read_from_block(block_index, 0, &mut block)?; + checksum.update(&block); + if checksum.finalize() != tag.checksum { + return Err( + CorruptKind::JournalDescriptorTagChecksum.into() + ); + } + + uncommitted_block_map.insert(tag.block_number, block_index); + } + } else if header.block_type == JournalBlockType::COMMIT { + if !is_commit_block_checksum_valid(superblock, &block) { + return Err(CorruptKind::JournalCommitBlockChecksum.into()); + } + + // Move the entries from `uncommitted_block_map` to `block_map`. + block_map.extend(uncommitted_block_map.iter()); + uncommitted_block_map.clear(); + + // TODO: unwrap + sequence = sequence.checked_add(1).unwrap(); + } else { + return Err( + Incompatible::JournalBlockType(header.block_type.0).into() + ); + } + } + + Ok(block_map) +} + +fn is_commit_block_checksum_valid( + superblock: &JournalSuperblock, + block: &[u8], +) -> bool { + // The kernel documentation says that fields 0xc and 0xd contain the + // checksum type and size, but this is not correct. If the + // superblock features include `CHECKSUM_V3`, the type/size fields + // are both zero. + + const CHECKSUM_OFFSET: usize = 16; + const CHECKSUM_SIZE: usize = 4; + + let expected_checksum = read_u32be(block, CHECKSUM_OFFSET); + + let mut checksum = Checksum::new(); + checksum.update(superblock.uuid.as_bytes()); + checksum.update(&block[..CHECKSUM_OFFSET]); + checksum.update(&[0; CHECKSUM_SIZE]); + checksum.update(&block[CHECKSUM_OFFSET + CHECKSUM_SIZE..]); + + checksum.finalize() == expected_checksum +} + +#[cfg(all(test, feature = "std"))] +mod tests { + use crate::test_util::load_compressed_filesystem; + use alloc::rc::Rc; + + #[test] + fn test_journal() { + let mut fs = + load_compressed_filesystem("test_disk_4k_block_journal.bin.zst"); + + let test_dir = "/dir500"; + + // With the journal in place, this directory exists. + assert!(fs.exists(test_dir).unwrap()); + + // Clear the journal, and verify that the directory no longer exists. + Rc::get_mut(&mut fs.0).unwrap().journal.block_map.clear(); + assert!(!fs.exists(test_dir).unwrap()); } } diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs new file mode 100644 index 0000000..925b5e3 --- /dev/null +++ b/src/journal/descriptor_block.rs @@ -0,0 +1,172 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use crate::checksum::Checksum; +use crate::error::{CorruptKind, Ext4Error}; +use crate::journal::superblock::JournalSuperblock; +use crate::util::{read_u32be, u64_from_hilo}; +use alloc::vec::Vec; +use bitflags::bitflags; + +pub(super) fn is_descriptor_block_checksum_valid( + superblock: &JournalSuperblock, + block: &[u8], +) -> bool { + // OK to unwrap: minimum block length is 1024. + let checksum_offset = block.len().checked_sub(4).unwrap(); + let expected_checksum = read_u32be(block, checksum_offset); + let mut checksum = Checksum::new(); + checksum.update(superblock.uuid.as_bytes()); + checksum.update(&block[..checksum_offset]); + checksum.update_u32_be(0); + + checksum.finalize() == expected_checksum +} + +#[derive(Debug, Eq, PartialEq)] +pub(super) struct DescriptorBlockTag { + pub(super) block_number: u64, + pub(super) flags: DescriptorBlockTagFlags, + + /// Checksum of the block data. + /// + /// Note that this checksum is for the data block at + /// `block_number`. The data in the tag itself is covered by the + /// descriptor block checksum. + pub(super) checksum: u32, +} + +impl DescriptorBlockTag { + const SIZE_WITHOUT_UUID: usize = 16; + const SIZE_WITH_UUID: usize = 32; + + /// Size (in bytes) of the tag when encoded in a block. + fn encoded_size(&self) -> usize { + if self.flags.contains(DescriptorBlockTagFlags::UUID_OMITTED) { + Self::SIZE_WITHOUT_UUID + } else { + Self::SIZE_WITH_UUID + } + } + + pub(super) fn read_bytes(bytes: &[u8]) -> Result { + // Note: the tag format depends on feature flags in the journal + // superblock. The code in this function is only correct if the + // `CHECKSUM_V3` feature is enabled (this is checked when + // loading the superblock). + + if bytes.len() < Self::SIZE_WITHOUT_UUID { + return Err(CorruptKind::JournalDescriptorTagTruncated.into()); + } + + let t_blocknr = read_u32be(bytes, 0); + let t_flags = read_u32be(bytes, 4); + let t_blocknr_high = read_u32be(bytes, 8); + let t_checksum = read_u32be(bytes, 12); + + let flags = DescriptorBlockTagFlags::from_bits_retain(t_flags); + + if !flags.contains(DescriptorBlockTagFlags::UUID_OMITTED) + && bytes.len() < Self::SIZE_WITH_UUID + { + return Err(CorruptKind::JournalDescriptorTagTruncated.into()); + } + + Ok(Self { + block_number: u64_from_hilo(t_blocknr_high, t_blocknr), + flags, + checksum: t_checksum, + }) + } + + // TODO: this could be an iterator instead of allocating. + pub(super) fn read_bytes_to_vec( + mut bytes: &[u8], + ) -> Result, Ext4Error> { + let mut v = Vec::new(); + + while !bytes.is_empty() { + let tag = Self::read_bytes(bytes)?; + let is_end = tag.flags.contains(DescriptorBlockTagFlags::LAST_TAG); + let size = tag.encoded_size(); + v.push(tag); + + if is_end { + return Ok(v); + } + + bytes = &bytes[size..]; + } + + Err(CorruptKind::JournalDescriptorBlockMissingLastTag.into()) + } +} + +bitflags! { + #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] + pub struct DescriptorBlockTagFlags: u32 { + const ESCAPED = 0x1; + const UUID_OMITTED = 0x2; + const DELETED = 0x4; + const LAST_TAG = 0x8; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn push_u32be(bytes: &mut Vec, value: u32) { + bytes.extend(&value.to_be_bytes()); + } + + #[test] + fn test_journal_descriptor_block_tags() { + let mut bytes = vec![]; + assert_eq!( + DescriptorBlockTag::read_bytes_to_vec(&bytes).unwrap_err(), + CorruptKind::JournalDescriptorBlockMissingLastTag + ); + + // Block number low. + push_u32be(&mut bytes, 0x1000); + // Flags. + push_u32be(&mut bytes, DescriptorBlockTagFlags::UUID_OMITTED.bits()); + // Block number high. + push_u32be(&mut bytes, 0xa000); + // Checksum. + push_u32be(&mut bytes, 0x123); + + // Block number low. + push_u32be(&mut bytes, 0x2000); + // Flags. + push_u32be(&mut bytes, DescriptorBlockTagFlags::LAST_TAG.bits()); + // Block number high. + push_u32be(&mut bytes, 0xb000); + // Checksum. + push_u32be(&mut bytes, 0x456); + // UUID. + bytes.extend([0; 16]); + + assert_eq!( + DescriptorBlockTag::read_bytes_to_vec(&bytes).unwrap(), + vec![ + DescriptorBlockTag { + block_number: 0xa000_0000_1000, + flags: DescriptorBlockTagFlags::UUID_OMITTED, + checksum: 0x123, + }, + DescriptorBlockTag { + block_number: 0xb000_0000_2000, + flags: DescriptorBlockTagFlags::LAST_TAG, + checksum: 0x456, + } + ] + ); + } +} diff --git a/src/journal/superblock.rs b/src/journal/superblock.rs index 32c8d8c..869f73d 100644 --- a/src/journal/superblock.rs +++ b/src/journal/superblock.rs @@ -190,6 +190,28 @@ fn check_incompat_features( #[cfg(all(test, feature = "std"))] mod tests { use super::*; + use crate::test_util::load_compressed_filesystem; + + #[test] + fn test_load_journal_superblock() { + let fs = + load_compressed_filesystem("test_disk_4k_block_journal.bin.zst"); + let journal_inode = + Inode::read(&fs, fs.0.superblock.journal_inode.unwrap()).unwrap(); + let superblock = JournalSuperblock::load(&fs, &journal_inode).unwrap(); + assert_eq!( + superblock, + JournalSuperblock { + block_size: 4096, + sequence: 3, + start_block: 289, + uuid: Uuid([ + 0x6c, 0x48, 0x4f, 0x1b, 0x7f, 0x71, 0x47, 0x4c, 0xa1, 0xf9, + 0x3b, 0x50, 0x0c, 0xc1, 0xe2, 0x74 + ]), + } + ); + } fn write_u32be(bytes: &mut [u8], offset: usize, value: u32) { bytes[offset..offset + 4].copy_from_slice(&value.to_be_bytes()); diff --git a/src/lib.rs b/src/lib.rs index 463d290..8dc9924 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -306,6 +306,8 @@ impl Ext4 { ) }; + let block_index = self.0.journal.map_block_index(block_index); + // The first 1024 bytes are reserved for non-filesystem // data. This conveniently allows for something like a null // pointer check. diff --git a/src/superblock.rs b/src/superblock.rs index 0292ee1..ee85d7b 100644 --- a/src/superblock.rs +++ b/src/superblock.rs @@ -121,22 +121,24 @@ impl Superblock { return Err(CorruptKind::InodeSize.into()); } - let journal_inode = - if compatible_features.contains(CompatibleFeatures::HAS_JOURNAL) { - // For now a separate journal device is not supported, so - // assert that feature is not present. This assert cannot - // fail because of the call to `check_incompat_features` - // above. - assert!(!incompatible_features - .contains(IncompatibleFeatures::SEPARATE_JOURNAL_DEVICE)); - - Some( - InodeIndex::new(s_journal_inum) - .ok_or(CorruptKind::JournalInode)?, - ) - } else { - None - }; + let journal_inode = if compatible_features + .contains(CompatibleFeatures::HAS_JOURNAL) + && incompatible_features.contains(IncompatibleFeatures::RECOVERY) + { + // For now a separate journal device is not supported, so + // assert that feature is not present. This assert cannot + // fail because of the call to `check_incompat_features` + // above. + assert!(!incompatible_features + .contains(IncompatibleFeatures::SEPARATE_JOURNAL_DEVICE)); + + Some( + InodeIndex::new(s_journal_inum) + .ok_or(CorruptKind::JournalInode)?, + ) + } else { + None + }; // Validate the superblock checksum. if read_only_compatible_features @@ -199,7 +201,6 @@ fn check_incompat_features( // relax some of these in the future. let required_features = IncompatibleFeatures::FILE_TYPE_IN_DIR_ENTRY; let disallowed_features = IncompatibleFeatures::COMPRESSION - | IncompatibleFeatures::RECOVERY | IncompatibleFeatures::SEPARATE_JOURNAL_DEVICE | IncompatibleFeatures::META_BLOCK_GROUPS | IncompatibleFeatures::MULTIPLE_MOUNT_PROTECTION @@ -394,10 +395,12 @@ mod tests { assert_eq!( check_incompat_features( - required | IncompatibleFeatures::RECOVERY.bits() + required | IncompatibleFeatures::SEPARATE_JOURNAL_DEVICE.bits() ) .unwrap_err(), - Incompatible::UnsupportedFeatures(IncompatibleFeatures::RECOVERY) + Incompatible::UnsupportedFeatures( + IncompatibleFeatures::SEPARATE_JOURNAL_DEVICE + ) ); } } From 0ecaa07f350f8c154e8c7228b2571ac7fc0c9cc5 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 6 Feb 2025 23:14:44 -0500 Subject: [PATCH 03/11] Add tag iter --- src/error.rs | 6 --- src/journal.rs | 15 ++++--- src/journal/descriptor_block.rs | 76 +++++++++++++++++++++------------ 3 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/error.rs b/src/error.rs index ccf650d..662b7c7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -250,9 +250,6 @@ pub(crate) enum CorruptKind { /// Journal descriptor tag checksum is invalid. JournalDescriptorTagChecksum, - /// Not enough bytes to read a journal descriptor tag. - JournalDescriptorTagTruncated, - /// Journal has a descriptor block that contains no tag with the last-tag flag set. JournalDescriptorBlockMissingLastTag, @@ -359,9 +356,6 @@ impl Display for CorruptKind { Self::JournalDescriptorTagChecksum => { write!(f, "journal descriptor tag checksum is invalid") } - Self::JournalDescriptorTagTruncated => { - write!(f, "journal descriptor tag truncated") - } Self::JournalDescriptorBlockMissingLastTag => write!( f, "a journal descriptor block has no tag with the last-tag flag set" diff --git a/src/journal.rs b/src/journal.rs index 69a5e3c..8ed6238 100644 --- a/src/journal.rs +++ b/src/journal.rs @@ -20,7 +20,7 @@ use alloc::collections::BTreeMap; use alloc::vec; use block_header::{JournalBlockHeader, JournalBlockType}; use descriptor_block::{ - is_descriptor_block_checksum_valid, DescriptorBlockTag, + is_descriptor_block_checksum_valid, DescriptorBlockTagIter, }; use superblock::JournalSuperblock; @@ -91,6 +91,7 @@ fn load_block_map( // journal when bad data is encountered. let mut block = vec![0; fs.0.superblock.block_size.to_usize()]; + let mut data_block = vec![0; fs.0.superblock.block_size.to_usize()]; let mut block_map = BTreeMap::new(); let mut uncommitted_block_map = BTreeMap::new(); let mut sequence = superblock.sequence; @@ -114,19 +115,21 @@ fn load_block_map( return Err(CorruptKind::JournalDescriptorBlockChecksum.into()); } - let tags = - DescriptorBlockTag::read_bytes_to_vec(&block[12..]).unwrap(); + let tags = DescriptorBlockTagIter::new(&block[12..]); + + for tag in tags { + let tag = tag?; - for tag in &tags { let block_index = journal_block_iter .next() .ok_or(CorruptKind::JournalTruncated)??; + // Check the data block checksum. let mut checksum = Checksum::new(); checksum.update(superblock.uuid.as_bytes()); checksum.update_u32_be(sequence); - fs.read_from_block(block_index, 0, &mut block)?; - checksum.update(&block); + fs.read_from_block(block_index, 0, &mut data_block)?; + checksum.update(&data_block); if checksum.finalize() != tag.checksum { return Err( CorruptKind::JournalDescriptorTagChecksum.into() diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 925b5e3..6476d66 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -10,7 +10,6 @@ use crate::checksum::Checksum; use crate::error::{CorruptKind, Ext4Error}; use crate::journal::superblock::JournalSuperblock; use crate::util::{read_u32be, u64_from_hilo}; -use alloc::vec::Vec; use bitflags::bitflags; pub(super) fn is_descriptor_block_checksum_valid( @@ -54,14 +53,14 @@ impl DescriptorBlockTag { } } - pub(super) fn read_bytes(bytes: &[u8]) -> Result { + pub(super) fn read_bytes(bytes: &[u8]) -> Option { // Note: the tag format depends on feature flags in the journal // superblock. The code in this function is only correct if the // `CHECKSUM_V3` feature is enabled (this is checked when // loading the superblock). if bytes.len() < Self::SIZE_WITHOUT_UUID { - return Err(CorruptKind::JournalDescriptorTagTruncated.into()); + return None; } let t_blocknr = read_u32be(bytes, 0); @@ -74,42 +73,67 @@ impl DescriptorBlockTag { if !flags.contains(DescriptorBlockTagFlags::UUID_OMITTED) && bytes.len() < Self::SIZE_WITH_UUID { - return Err(CorruptKind::JournalDescriptorTagTruncated.into()); + return None; } - Ok(Self { + Some(Self { block_number: u64_from_hilo(t_blocknr_high, t_blocknr), flags, checksum: t_checksum, }) } +} + +pub(super) struct DescriptorBlockTagIter<'a> { + bytes: &'a [u8], + is_done: bool, +} - // TODO: this could be an iterator instead of allocating. - pub(super) fn read_bytes_to_vec( - mut bytes: &[u8], - ) -> Result, Ext4Error> { - let mut v = Vec::new(); +impl<'a> DescriptorBlockTagIter<'a> { + pub(super) fn new(bytes: &'a [u8]) -> Self { + Self { + bytes, + is_done: false, + } + } +} - while !bytes.is_empty() { - let tag = Self::read_bytes(bytes)?; - let is_end = tag.flags.contains(DescriptorBlockTagFlags::LAST_TAG); - let size = tag.encoded_size(); - v.push(tag); +impl Iterator for DescriptorBlockTagIter<'_> { + type Item = Result; - if is_end { - return Ok(v); - } + fn next(&mut self) -> Option { + if self.is_done { + return None; + } - bytes = &bytes[size..]; + let tag = if let Some(tag) = DescriptorBlockTag::read_bytes(self.bytes) + { + tag + } else { + // If there were not enough bytes left to read the next tag, + // then the expected tag with the LAST_TAG flag set is missing. + self.is_done = true; + return Some(Err( + CorruptKind::JournalDescriptorBlockMissingLastTag.into(), + )); + }; + + if tag.flags.contains(DescriptorBlockTagFlags::LAST_TAG) { + // Last tag reached, nothing more to read. + self.is_done = true; + return Some(Ok(tag)); } - Err(CorruptKind::JournalDescriptorBlockMissingLastTag.into()) + // Update the remaining bytes. + self.bytes = &self.bytes[tag.encoded_size()..]; + + Some(Ok(tag)) } } bitflags! { #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] - pub struct DescriptorBlockTagFlags: u32 { + pub(crate) struct DescriptorBlockTagFlags: u32 { const ESCAPED = 0x1; const UUID_OMITTED = 0x2; const DELETED = 0x4; @@ -128,10 +152,6 @@ mod tests { #[test] fn test_journal_descriptor_block_tags() { let mut bytes = vec![]; - assert_eq!( - DescriptorBlockTag::read_bytes_to_vec(&bytes).unwrap_err(), - CorruptKind::JournalDescriptorBlockMissingLastTag - ); // Block number low. push_u32be(&mut bytes, 0x1000); @@ -154,8 +174,10 @@ mod tests { bytes.extend([0; 16]); assert_eq!( - DescriptorBlockTag::read_bytes_to_vec(&bytes).unwrap(), - vec![ + DescriptorBlockTagIter::new(&bytes) + .map(Result::unwrap) + .collect::>(), + [ DescriptorBlockTag { block_number: 0xa000_0000_1000, flags: DescriptorBlockTagFlags::UUID_OMITTED, From fd4866270336970e9bbe536341578a6622d0d84d Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 6 Feb 2025 23:30:01 -0500 Subject: [PATCH 04/11] improve tests --- src/journal/descriptor_block.rs | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 6476d66..4ab40ff 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -150,7 +150,7 @@ mod tests { } #[test] - fn test_journal_descriptor_block_tags() { + fn test_descriptor_block_tag_iter() { let mut bytes = vec![]; // Block number low. @@ -191,4 +191,38 @@ mod tests { ] ); } + + #[test] + fn test_descriptor_block_tag_iter_empty() { + let bytes = vec![]; + assert_eq!( + DescriptorBlockTagIter::new(&bytes) + .next() + .unwrap() + .unwrap_err(), + CorruptKind::JournalDescriptorBlockMissingLastTag + ); + } + + #[test] + fn test_descriptor_block_tag_iter_missing_uuid() { + let mut bytes = vec![]; + + // Block number low. + push_u32be(&mut bytes, 0x2000); + // Flags. + push_u32be(&mut bytes, 0); + // Block number high. + push_u32be(&mut bytes, 0xb000); + // Checksum. + push_u32be(&mut bytes, 0x456); + + assert_eq!( + DescriptorBlockTagIter::new(&bytes) + .next() + .unwrap() + .unwrap_err(), + CorruptKind::JournalDescriptorBlockMissingLastTag + ); + } } From 8c88d902b26dba431d418b0a7d067996a335f160 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 6 Feb 2025 23:31:31 -0500 Subject: [PATCH 05/11] more private --- src/journal/descriptor_block.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 4ab40ff..26fe89d 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -30,7 +30,7 @@ pub(super) fn is_descriptor_block_checksum_valid( #[derive(Debug, Eq, PartialEq)] pub(super) struct DescriptorBlockTag { pub(super) block_number: u64, - pub(super) flags: DescriptorBlockTagFlags, + flags: DescriptorBlockTagFlags, /// Checksum of the block data. /// @@ -53,7 +53,7 @@ impl DescriptorBlockTag { } } - pub(super) fn read_bytes(bytes: &[u8]) -> Option { + fn read_bytes(bytes: &[u8]) -> Option { // Note: the tag format depends on feature flags in the journal // superblock. The code in this function is only correct if the // `CHECKSUM_V3` feature is enabled (this is checked when @@ -133,7 +133,7 @@ impl Iterator for DescriptorBlockTagIter<'_> { bitflags! { #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] - pub(crate) struct DescriptorBlockTagFlags: u32 { + struct DescriptorBlockTagFlags: u32 { const ESCAPED = 0x1; const UUID_OMITTED = 0x2; const DELETED = 0x4; From 20a5c2f89e75ad75f733d7f88b4e3500cf74d52e Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Thu, 6 Feb 2025 23:34:10 -0500 Subject: [PATCH 06/11] add todo --- src/journal/descriptor_block.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 26fe89d..b7a8ba9 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -12,6 +12,7 @@ use crate::journal::superblock::JournalSuperblock; use crate::util::{read_u32be, u64_from_hilo}; use bitflags::bitflags; +// TODO: return Result pub(super) fn is_descriptor_block_checksum_valid( superblock: &JournalSuperblock, block: &[u8], From e903216d09a0bb7963a27a67dc470d7487232c16 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Fri, 7 Feb 2025 02:48:20 -0500 Subject: [PATCH 07/11] resolve todo --- src/journal.rs | 20 ++++++++++---------- src/journal/descriptor_block.rs | 11 +++++++---- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/journal.rs b/src/journal.rs index 8ed6238..282f084 100644 --- a/src/journal.rs +++ b/src/journal.rs @@ -20,7 +20,7 @@ use alloc::collections::BTreeMap; use alloc::vec; use block_header::{JournalBlockHeader, JournalBlockType}; use descriptor_block::{ - is_descriptor_block_checksum_valid, DescriptorBlockTagIter, + validate_descriptor_block_checksum, DescriptorBlockTagIter, }; use superblock::JournalSuperblock; @@ -111,9 +111,7 @@ fn load_block_map( } if header.block_type == JournalBlockType::DESCRIPTOR { - if !is_descriptor_block_checksum_valid(superblock, &block) { - return Err(CorruptKind::JournalDescriptorBlockChecksum.into()); - } + validate_descriptor_block_checksum(superblock, &block)?; let tags = DescriptorBlockTagIter::new(&block[12..]); @@ -139,9 +137,7 @@ fn load_block_map( uncommitted_block_map.insert(tag.block_number, block_index); } } else if header.block_type == JournalBlockType::COMMIT { - if !is_commit_block_checksum_valid(superblock, &block) { - return Err(CorruptKind::JournalCommitBlockChecksum.into()); - } + validate_commit_block_checksum(superblock, &block)?; // Move the entries from `uncommitted_block_map` to `block_map`. block_map.extend(uncommitted_block_map.iter()); @@ -159,10 +155,10 @@ fn load_block_map( Ok(block_map) } -fn is_commit_block_checksum_valid( +fn validate_commit_block_checksum( superblock: &JournalSuperblock, block: &[u8], -) -> bool { +) -> Result<(), Ext4Error> { // The kernel documentation says that fields 0xc and 0xd contain the // checksum type and size, but this is not correct. If the // superblock features include `CHECKSUM_V3`, the type/size fields @@ -179,7 +175,11 @@ fn is_commit_block_checksum_valid( checksum.update(&[0; CHECKSUM_SIZE]); checksum.update(&block[CHECKSUM_OFFSET + CHECKSUM_SIZE..]); - checksum.finalize() == expected_checksum + if checksum.finalize() == expected_checksum { + Ok(()) + } else { + Err(CorruptKind::JournalCommitBlockChecksum.into()) + } } #[cfg(all(test, feature = "std"))] diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index b7a8ba9..7f45ec1 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -12,11 +12,10 @@ use crate::journal::superblock::JournalSuperblock; use crate::util::{read_u32be, u64_from_hilo}; use bitflags::bitflags; -// TODO: return Result -pub(super) fn is_descriptor_block_checksum_valid( +pub(super) fn validate_descriptor_block_checksum( superblock: &JournalSuperblock, block: &[u8], -) -> bool { +) -> Result<(), Ext4Error> { // OK to unwrap: minimum block length is 1024. let checksum_offset = block.len().checked_sub(4).unwrap(); let expected_checksum = read_u32be(block, checksum_offset); @@ -25,7 +24,11 @@ pub(super) fn is_descriptor_block_checksum_valid( checksum.update(&block[..checksum_offset]); checksum.update_u32_be(0); - checksum.finalize() == expected_checksum + if checksum.finalize() == expected_checksum { + Ok(()) + } else { + Err(CorruptKind::JournalDescriptorBlockChecksum.into()) + } } #[derive(Debug, Eq, PartialEq)] From 63b85b31853e9ce90c92026c913a726f49c64c2c Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Fri, 7 Feb 2025 02:51:49 -0500 Subject: [PATCH 08/11] improve docs --- src/journal/descriptor_block.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 7f45ec1..2f80421 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -33,15 +33,18 @@ pub(super) fn validate_descriptor_block_checksum( #[derive(Debug, Eq, PartialEq)] pub(super) struct DescriptorBlockTag { - pub(super) block_number: u64, - flags: DescriptorBlockTagFlags, + /// Absolute block index in the filesystem that should be replaced + /// with the data block associated with this tag. + pub(super) block_index: u64, /// Checksum of the block data. /// - /// Note that this checksum is for the data block at - /// `block_number`. The data in the tag itself is covered by the + /// Note that this checksum is for the data block associated with + /// this tag. The data in the tag itself is covered by the /// descriptor block checksum. pub(super) checksum: u32, + + flags: DescriptorBlockTagFlags, } impl DescriptorBlockTag { From f5792d0d8c9f5e63b1c0edf7583174d26626b4ce Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Fri, 7 Feb 2025 02:55:19 -0500 Subject: [PATCH 09/11] improve docs --- src/journal/descriptor_block.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 2f80421..98059b4 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -31,6 +31,13 @@ pub(super) fn validate_descriptor_block_checksum( } } +/// Data block tag within a descriptor block. +/// +/// Each descriptor block contains an array of tags, one for each data +/// block following the descriptor block. Each data block will replace +/// replace a block within the ext4 filesystem. The tag indicates where +/// the data block maps into the filesystem, and provides a checksum for +/// the data block. #[derive(Debug, Eq, PartialEq)] pub(super) struct DescriptorBlockTag { /// Absolute block index in the filesystem that should be replaced @@ -141,6 +148,7 @@ impl Iterator for DescriptorBlockTagIter<'_> { bitflags! { #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] struct DescriptorBlockTagFlags: u32 { + // TODO: Incompat for now! const ESCAPED = 0x1; const UUID_OMITTED = 0x2; const DELETED = 0x4; From c38640bd43490a6479a4c8e057d8b4225b710674 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sun, 9 Feb 2025 15:48:02 -0500 Subject: [PATCH 10/11] checkpoint --- src/error.rs | 6 ++++++ src/journal.rs | 2 +- src/journal/descriptor_block.rs | 15 ++++++++++----- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/error.rs b/src/error.rs index 662b7c7..ab4fbfd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -512,6 +512,9 @@ pub enum Incompatible { /// Raw journal block type. u32, ), + + /// The journal contains an escaped block. + JournalBlockEscaped, } impl Display for Incompatible { @@ -535,6 +538,9 @@ impl Display for Incompatible { Self::JournalBlockType(val) => { write!(f, "journal block type is not supported: {val}") } + Self::JournalBlockEscaped => { + write!(f, "journal contains an escaped data block") + } Self::JournalChecksumType(val) => { write!(f, "journal checksum type is not supported: {val}") } diff --git a/src/journal.rs b/src/journal.rs index 282f084..42d8ad6 100644 --- a/src/journal.rs +++ b/src/journal.rs @@ -134,7 +134,7 @@ fn load_block_map( ); } - uncommitted_block_map.insert(tag.block_number, block_index); + uncommitted_block_map.insert(tag.block_index, block_index); } } else if header.block_type == JournalBlockType::COMMIT { validate_commit_block_checksum(superblock, &block)?; diff --git a/src/journal/descriptor_block.rs b/src/journal/descriptor_block.rs index 98059b4..32c43d1 100644 --- a/src/journal/descriptor_block.rs +++ b/src/journal/descriptor_block.rs @@ -7,7 +7,7 @@ // except according to those terms. use crate::checksum::Checksum; -use crate::error::{CorruptKind, Ext4Error}; +use crate::error::{CorruptKind, Ext4Error, Incompatible}; use crate::journal::superblock::JournalSuperblock; use crate::util::{read_u32be, u64_from_hilo}; use bitflags::bitflags; @@ -91,7 +91,7 @@ impl DescriptorBlockTag { } Some(Self { - block_number: u64_from_hilo(t_blocknr_high, t_blocknr), + block_index: u64_from_hilo(t_blocknr_high, t_blocknr), flags, checksum: t_checksum, }) @@ -132,6 +132,12 @@ impl Iterator for DescriptorBlockTagIter<'_> { )); }; + // Escaped data blocks are not yet supported. + if tag.flags.contains(DescriptorBlockTagFlags::ESCAPED) { + self.is_done = true; + return Some(Err(Incompatible::JournalBlockEscaped.into())); + } + if tag.flags.contains(DescriptorBlockTagFlags::LAST_TAG) { // Last tag reached, nothing more to read. self.is_done = true; @@ -148,7 +154,6 @@ impl Iterator for DescriptorBlockTagIter<'_> { bitflags! { #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] struct DescriptorBlockTagFlags: u32 { - // TODO: Incompat for now! const ESCAPED = 0x1; const UUID_OMITTED = 0x2; const DELETED = 0x4; @@ -194,12 +199,12 @@ mod tests { .collect::>(), [ DescriptorBlockTag { - block_number: 0xa000_0000_1000, + block_index: 0xa000_0000_1000, flags: DescriptorBlockTagFlags::UUID_OMITTED, checksum: 0x123, }, DescriptorBlockTag { - block_number: 0xb000_0000_2000, + block_index: 0xb000_0000_2000, flags: DescriptorBlockTagFlags::LAST_TAG, checksum: 0x456, } From a83f11ca04eefda1e0bd773d4732943468407bdc Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Sun, 9 Feb 2025 18:23:07 -0500 Subject: [PATCH 11/11] sequence overflow --- src/error.rs | 4 ++++ src/journal.rs | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index ab4fbfd..3e642fa 100644 --- a/src/error.rs +++ b/src/error.rs @@ -253,6 +253,9 @@ pub(crate) enum CorruptKind { /// Journal has a descriptor block that contains no tag with the last-tag flag set. JournalDescriptorBlockMissingLastTag, + /// Journal sequence number overflowed. + JournalSequenceOverflow, + /// An inode's checksum is invalid. InodeChecksum(InodeIndex), @@ -360,6 +363,7 @@ impl Display for CorruptKind { f, "a journal descriptor block has no tag with the last-tag flag set" ), + Self::JournalSequenceOverflow => write!(f, "journal sequence number overflowed"), Self::InodeChecksum(inode) => { write!(f, "invalid checksum for inode {inode}") } diff --git a/src/journal.rs b/src/journal.rs index 42d8ad6..8c93a00 100644 --- a/src/journal.rs +++ b/src/journal.rs @@ -143,8 +143,9 @@ fn load_block_map( block_map.extend(uncommitted_block_map.iter()); uncommitted_block_map.clear(); - // TODO: unwrap - sequence = sequence.checked_add(1).unwrap(); + sequence = sequence + .checked_add(1) + .ok_or(CorruptKind::JournalSequenceOverflow)?; } else { return Err( Incompatible::JournalBlockType(header.block_type.0).into()