From f18ecd8a6523326a29e30ffc312cdb8237db4945 Mon Sep 17 00:00:00 2001 From: Daniel Maslowski Date: Thu, 27 Nov 2025 12:46:17 +0100 Subject: [PATCH] lib + dir: fix Clippy issues and rework parsing - get rid of unwrap()s - add more bounds checks Signed-off-by: Daniel Maslowski --- src/dir/gen2.rs | 68 +++++++++++++++++++++++++++++-------------------- src/dir/gen3.rs | 25 +++++++++--------- src/lib.rs | 10 ++++---- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/src/dir/gen2.rs b/src/dir/gen2.rs index e1e81b7..a016db1 100644 --- a/src/dir/gen2.rs +++ b/src/dir/gen2.rs @@ -105,7 +105,7 @@ impl Entry { let b = self.mod_base; let rapi = self.flags.rapi(); let kapi = self.flags.kapi(); - let code_start = (b as usize + (rapi + kapi) * 0x1000) as usize; + let code_start = b as usize + (rapi + kapi) * 0x1000; let code_end = (b + self.code_size) as usize; let data_end = (b + self.memory_size) as usize; BinaryMap { @@ -215,15 +215,15 @@ impl Display for Directory { } } -const HEADER_SIZE: usize = core::mem::size_of::
(); - impl Directory { pub fn new(data: &[u8], offset: usize) -> Result { - let Ok(manifest) = Manifest::new(data) else { - return Err("cannot parse Gen 2 directory manifest".to_string()); + let manifest = match Manifest::new(data) { + Ok(r) => r, + Err(e) => return Err(format!("cannot parse Gen 2 directory manifest: {e:?}")), }; - let Ok((header, _)) = Header::read_from_prefix(&manifest.mdata) else { - return Err("cannot parse ME FW Gen 2 directory header".to_string()); + let (header, rest) = match Header::read_from_prefix(&manifest.mdata) { + Ok((h, rest)) => (h, rest), + Err(e) => return Err(format!("cannot parse Gen 2 directory header: {e:?}")), }; let name = match from_utf8(&header.name) { Ok(n) => n.trim_end_matches('\0').to_string(), @@ -231,9 +231,7 @@ impl Directory { }; // Check magic bytes of first entry. - let pos = HEADER_SIZE; - let slice = &manifest.mdata[pos..]; - let m = &slice[..4]; + let m = &rest[..4]; if !m.eq(MODULE_MAGIC_BYTES) { return Err(format!( "entry magic not found, got {m:02x?}, wanted {MODULE_MAGIC_BYTES:02x?} ({MODULE_MAGIC})" @@ -241,8 +239,9 @@ impl Directory { } // Parse the entries themselves. let count = manifest.header.entries as usize; - let Ok((r, _)) = Ref::<_, [Entry]>::from_prefix_with_elems(slice, count) else { - return Err(format!("cannot parse ME FW Gen 2 directory entries",)); + let r = match Ref::<_, [Entry]>::from_prefix_with_elems(rest, count) { + Ok((r, _)) => r, + Err(e) => return Err(format!("cannot parse Gen 2 directory entries: {e}")), }; let entries = r.to_vec(); @@ -265,16 +264,28 @@ impl Directory { "Expected {SIG_LUT_BYTES:02x?} @ {o:08x}, got {sig:02x?}" ))); } - let (header, _) = LutHeader::read_from_prefix(&data[o..]).unwrap(); + let header = match LutHeader::read_from_prefix(&data[o..]) { + Ok((r, _)) => r, + Err(e) => return Module::Huffman(Err(format!("{e:?}"))), + }; let count = header.chunk_count as usize; let lo = o + LUT_HEADER_SIZE; - let (lut, _) = - Ref::<_, [u32]>::from_prefix_with_elems(&data[lo..], count).unwrap(); - let huff = HuffmanModule { - header, - chunks: lut.to_vec(), - }; - Module::Huffman(Ok((*e, huff))) + let l = data.len(); + if lo > l { + return Module::Huffman(Err(format!( + "offset {lo:08x} out of bounds ({l})" + ))); + } + match Ref::<_, [u32]>::from_prefix_with_elems(&data[lo..], count) { + Ok((lut, _)) => { + let huff = HuffmanModule { + header, + chunks: lut.to_vec(), + }; + Module::Huffman(Ok((*e, huff))) + } + Err(e) => Module::Huffman(Err(format!("{e}"))), + } } Compression::Lzma => { if sig != SIG_LZMA_BYTES { @@ -319,7 +330,7 @@ impl Directory { Err("new offset cannot be calculated".into()) } - fn dump_ranges(ranges: &Vec>) { + fn dump_ranges(ranges: &[Range]) { let group_size = 4; for (i, r) in ranges.iter().enumerate() { if i % group_size == group_size - 1 { @@ -332,7 +343,7 @@ impl Directory { } /// Get the offset ranges of the chunks. - fn chunks_as_ranges(&self, chunks: &Vec, stream_end: usize) -> Vec> { + fn chunks_as_ranges(&self, chunks: &[u32], stream_end: usize) -> Vec> { // NOTE: This is the end of the directory. // me_cleaner uses the end of the ME region. let dir_end = self.offset + self.size; @@ -352,6 +363,7 @@ impl Directory { } }) .collect::>(); + // This should move dir_end to the end. nonzero_offsets.sort(); // Turn offsets into ranges by finding the offset of the next chunk. offsets @@ -360,9 +372,11 @@ impl Directory { let o = *offset; let e = if o != 0 { // NOTE: nonzero_offsets are a subset of offsets, so this should never fail. + #[allow(clippy::unwrap_used)] let p = nonzero_offsets.iter().position(|e| *e == o).unwrap(); let next = p + 1; - // The last entry has no successor. + // The last entry potentially has no successor. It would be + // dir_end, which is the initial entry in nonzero_offsets. if next < nonzero_offsets.len() { nonzero_offsets[next] } else { @@ -395,7 +409,7 @@ impl Directory { if (*c >> 24) as u8 == CHUNK_INACTIVE { // } else { - *c = *c - offset_diff; + *c -= offset_diff; }; } return Ok(()); @@ -415,7 +429,7 @@ impl Directory { .find(|m| matches!(m, Module::Huffman(Ok(_)))) { let lut_header_offset = e.offset as usize; - return Some((lut_header_offset, &h)); + return Some((lut_header_offset, h)); } None } @@ -423,7 +437,7 @@ impl Directory { impl Removables for Directory { /// Removable ranges relative to the start of the Directory - fn removables(&self, retention_list: &Vec) -> Vec> { + fn removables(&self, retention_list: &[String]) -> Vec> { use log::{debug, info, warn}; let debug = false; let mut removables = vec![]; @@ -445,7 +459,7 @@ impl Removables for Directory { // NOTE: The header is always the same, since multiple // Huffman-encoded modules point to the same offset. let cs = h.header.chunk_size; - if all_chunks.len() == 0 { + if all_chunks.is_empty() { info!("Huffman chunk size: {cs}"); let stream_end = (h.header.hs0 + h.header.hs1) as usize; all_chunks = self.chunks_as_ranges(&h.chunks, stream_end); diff --git a/src/dir/gen3.rs b/src/dir/gen3.rs index 815a964..aba8a45 100644 --- a/src/dir/gen3.rs +++ b/src/dir/gen3.rs @@ -135,13 +135,13 @@ impl Display for CodePartitionDirectory { }; format!("{m}\n{kh}{me}") } - Err(e) => format!("{e}"), + Err(e) => e.to_string(), }; - let l3 = format!(" file name offset end size kind"); - write!(f, "{l1}\n{l2}\n{l3}\n").unwrap(); + let l3 = " file name offset end size kind".to_string(); + write!(f, "{l1}\n{l2}\n{l3}\n")?; let sorted_entries = self.sorted_entries(); for e in sorted_entries { - write!(f, " {e}\n").unwrap(); + writeln!(f, " {e}")?; } write!(f, "") } @@ -149,7 +149,7 @@ impl Display for CodePartitionDirectory { impl CodePartitionDirectory { pub fn new(data: &[u8], offset: usize) -> Result { - let Ok((header, _)) = CPDHeader::read_from_prefix(&data) else { + let Ok((header, _)) = CPDHeader::read_from_prefix(data) else { return Err("could not parse CPD header".to_string()); }; let name = header.name(); @@ -203,7 +203,7 @@ impl CodePartitionDirectory { impl Removables for CodePartitionDirectory { /// Removable ranges relative to the start of the directory - fn removables(&self, retention_list: &Vec) -> Vec> { + fn removables(&self, retention_list: &[String]) -> Vec> { use log::info; let mut removables = vec![]; @@ -224,12 +224,13 @@ impl Removables for CodePartitionDirectory { } // Remaining space to free after last entry let sorted = self.sorted_entries(); - let last = sorted.last().unwrap(); - let end = (last.flags_and_offset.offset() + last.size) as usize; - let o = end.next_multiple_of(FOUR_K); - let e = self.size; - removables.push(o..e); - info!("Remaining space: {o:08x}..{e:08x}"); + if let Some(last) = sorted.last() { + let end = (last.flags_and_offset.offset() + last.size) as usize; + let o = end.next_multiple_of(FOUR_K); + let e = self.size; + removables.push(o..e); + info!("Remaining space: {o:08x}..{e:08x}"); + } removables } } diff --git a/src/lib.rs b/src/lib.rs index eafe585..777f311 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,7 @@ pub const EMPTY: u8 = 0xff; pub trait Removables { /// Get removable ranges relative to the start of a section or directory. /// The respective section/directory needs to know its own offset. - fn removables(&self, retention_list: &Vec) -> Vec>; + fn removables(&self, retention_list: &[String]) -> Vec>; } #[derive(Serialize, Deserialize, Clone, Debug)] @@ -34,7 +34,7 @@ pub struct Firmware { impl Firmware { pub fn parse(data: &[u8], debug: bool) -> Self { - let ifd = IFD::parse(&data); + let ifd = IFD::parse(data); let me = match &ifd { Ok(ifd) => { let me_region = ifd.regions.me_range(); @@ -52,9 +52,9 @@ impl Firmware { } pub fn scan(data: &[u8], debug: bool) -> Self { - let ifd = IFD::parse(&data); - let me = ME::scan(&data, debug); - let fit = Fit::new(&data); + let ifd = IFD::parse(data); + let me = ME::scan(data, debug); + let fit = Fit::new(data); Self { ifd, me, fit } } }