From 4b75240f9eb203e0b91ed55076bd6b71bdde9eef Mon Sep 17 00:00:00 2001 From: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> Date: Mon, 16 Mar 2026 18:48:53 +0100 Subject: [PATCH 1/4] refactor(file): replace fdisk with gptman+mbrman Replace fragile fdisk-based partition detection with Rust-native crates: - add gptman (GPT with CRC32 validation) and mbrman (MBR including logical partitions) as dependencies - add src/file/partition.rs: get_partition_data() and is_gpt() - remove locale-sensitive fdisk shell invocation and regex parsing - mbrman iter() correctly reaches logical partitions 5 and 6 - fix pre-existing bug: PartitionInfo.end (last sector) was used as dd count= instead of sector count; rename field to count with correct semantics (ending_lba - starting_lba + 1 for GPT, p.sectors for MBR) - remove fdisk from package.metadata.deb depends - change PartitionInfo fields from String to typed u32/u64 Signed-off-by: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> --- Cargo.lock | 101 +++++++++++++++++++++++++++++++++++++++++- Cargo.toml | 4 +- src/file/functions.rs | 71 +++++++++-------------------- src/file/mod.rs | 1 + src/file/partition.rs | 50 +++++++++++++++++++++ 5 files changed, 175 insertions(+), 52 deletions(-) create mode 100644 src/file/partition.rs diff --git a/Cargo.lock b/Cargo.lock index 9d61789..3f3b746 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -520,12 +520,33 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "bitflags" version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" +[[package]] +name = "bitvec" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1bc2832c24239b0141d5674bb9174f9d68a8b5b3f2753311927c172ca46f7e9c" +dependencies = [ + "funty", + "radium", + "tap", + "wyz", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -757,6 +778,21 @@ dependencies = [ "libc", ] +[[package]] +name = "crc" +version = "3.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5eb8a2a1cd12ab0d987a5d5e825195d372001a4094a0376319d5a0ad71c1ba0d" +dependencies = [ + "crc-catalog", +] + +[[package]] +name = "crc-catalog" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "19d374276b40fb8bbdee95aef7c7fa6b5316ec764510eb64b8dd0e2ed0d7e7f5" + [[package]] name = "crc32fast" version = "1.5.0" @@ -1076,6 +1112,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "funty" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5a32815ae3f33302d95fdcb2ce17862f8c65363dcfd29360480ba1001fc9c" + [[package]] name = "futures" version = "0.3.32" @@ -1269,6 +1311,18 @@ dependencies = [ "syn", ] +[[package]] +name = "gptman" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d344ce89f41a128e74337a4830507927f1f378b4ae69a26d949d1d1cce55c048" +dependencies = [ + "bincode", + "crc", + "serde", + "thiserror 1.0.69", +] + [[package]] name = "h2" version = "0.3.27" @@ -1878,6 +1932,19 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "mbrman" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1fc3bff63c208d4a14301c6cb807af2d1a0760052584ce3f9a737b55fb85498" +dependencies = [ + "bincode", + "bitvec", + "serde", + "serde-big-array", + "thiserror 1.0.69", +] + [[package]] name = "memchr" version = "2.8.0" @@ -2015,10 +2082,12 @@ dependencies = [ "file_diff", "filemagic", "flate2", + "gptman", "httpmock", "keyring", "libfs", "log", + "mbrman", "num_cpus", "oauth2 5.0.0", "omnect-crypto", @@ -2370,6 +2439,12 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "radium" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" + [[package]] name = "rand" version = "0.7.3" @@ -2712,6 +2787,15 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-big-array" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11fc7cc2c76d73e0f27ee52abbd64eec84d46f370c88371120433196934e4b7f" +dependencies = [ + "serde", +] + [[package]] name = "serde_core" version = "1.0.228" @@ -2962,6 +3046,12 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "tap" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" + [[package]] name = "tar" version = "0.4.44" @@ -2980,7 +3070,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand 2.3.0", - "getrandom 0.4.2", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2", @@ -3746,6 +3836,15 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" +[[package]] +name = "wyz" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" +dependencies = [ + "tap", +] + [[package]] name = "xattr" version = "1.6.1" diff --git a/Cargo.toml b/Cargo.toml index 8e8cea2..9b4c11e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,8 @@ filemagic = { version = "0.13", default-features = false, features = [ "pkg-config", ] } flate2 = { version = "1.1", default-features = false } +gptman = { version = "1.1", default-features = false } +mbrman = { version = "0.5", default-features = false } omnect-crypto = { git = "https://github.com/omnect/omnect-crypto.git", tag = "0.4.0" } keyring = { version = "3.6", default-features = false } libfs = { version = "0.9", default-features = false } @@ -85,5 +87,5 @@ tar = "0.4" # metadata for building with cargo-deb (https://crates.io/crates/cargo-deb) [package.metadata.deb] -depends = "bmap-tools, e2tools, fdisk, keychain, libc6 (>= 2.34), libmagic1, libssl3 (>= 3.0.0), mtools" +depends = "bmap-tools, e2tools, keychain, libc6 (>= 2.34), libmagic1, libssl3 (>= 3.0.0), mtools" revision = "" diff --git a/src/file/functions.rs b/src/file/functions.rs index 2670a84..0e209cf 100644 --- a/src/file/functions.rs +++ b/src/file/functions.rs @@ -1,6 +1,5 @@ use anyhow::{Context, Result}; use log::{debug, warn}; -use regex::Regex; use std::collections::HashMap; use std::fmt::{self, Display}; use std::fs; @@ -22,9 +21,9 @@ pub enum Partition { #[derive(Debug)] struct PartitionInfo { - num: String, - start: String, - end: String, + num: u32, + start: u64, + count: u64, } impl Display for Partition { @@ -381,59 +380,31 @@ pub fn read_file_from_image( } fn get_partition_info(image_file: &str, partition: &Partition) -> Result { - let mut fdisk = Command::new("fdisk"); - fdisk - .arg("-l") - .arg("-o") - .arg("Device,Start,End") - .arg(image_file); - let fdisk_out = exec_cmd_with_output!(fdisk); + use crate::file::partition::{get_partition_data, is_gpt}; + + let gpt = is_gpt(image_file) + .context("get_partition_info: failed to detect partition table type")?; - let partition_num = match partition { + let partition_num: u32 = match partition { Partition::boot => 1, Partition::rootA => 2, - p @ (Partition::factory | Partition::cert) => { - let re = Regex::new(r"Disklabel type: (\D{3})").unwrap(); - - let matches = re - .captures(&fdisk_out) - .context("get_partition_info: regex no matches found")?; - anyhow::ensure!( - matches.len() == 2, - "'get_partition_info: regex contains unexpected number of matches" - ); - - let partition_type = &matches[1]; - - debug!("partition type: {partition_type}"); - - match (p, partition_type) { - (Partition::factory, "gpt") => 4, - (Partition::factory, "dos") => 5, - (Partition::cert, "gpt") => 5, - (Partition::cert, "dos") => 6, - _ => anyhow::bail!("get_partition_info: unhandled partition type"), - } + Partition::factory => { + if gpt { 4 } else { 5 } + } + Partition::cert => { + if gpt { 5 } else { 6 } } }; - let re = Regex::new(format!(r"{image_file}{partition_num}\s+(\d+)\s+(\d+)").as_str()) - .context("get_partition_info: failed to create regex")?; - - let matches = re - .captures(&fdisk_out) - .context("get_partition_info: regex no matches found")?; - anyhow::ensure!( - matches.len() == 3, - "'get_partition_info: regex contains unexpected number of matches" - ); + debug!("get_partition_info: partition={partition}, num={partition_num}, gpt={gpt}"); - let partition_offset = (matches[1].to_string(), matches[2].to_string()); + let data = get_partition_data(image_file, partition_num) + .with_context(|| format!("get_partition_info: failed to read partition {partition_num}"))?; let info = PartitionInfo { - num: partition_num.to_string(), - start: partition_offset.0, - end: partition_offset.1, + num: data.num, + start: data.start, + count: data.count, }; debug!("get_partition_info: {:?}", info); @@ -455,7 +426,7 @@ fn read_partition( .arg(format!("of={partition_file}")) .arg("bs=512") .arg(format!("skip={}", partition_info.start)) - .arg(format!("count={}", partition_info.end)) + .arg(format!("count={}", partition_info.count)) .arg("conv=sparse") .arg("status=none"); exec_cmd!(dd); @@ -476,7 +447,7 @@ fn write_partition( .arg(format!("of={image_file}")) .arg("bs=512") .arg(format!("seek={}", partition_info.start)) - .arg(format!("count={}", partition_info.end)) + .arg(format!("count={}", partition_info.count)) .arg("conv=notrunc,sparse") .arg("status=none"); exec_cmd!(dd); diff --git a/src/file/mod.rs b/src/file/mod.rs index fbe60e8..6f9db48 100644 --- a/src/file/mod.rs +++ b/src/file/mod.rs @@ -1,5 +1,6 @@ pub mod compression; pub mod functions; +mod partition; use super::validators::{ device_update, identity::{IdentityConfig, IdentityType, validate_identity}, diff --git a/src/file/partition.rs b/src/file/partition.rs new file mode 100644 index 0000000..24dd2d8 --- /dev/null +++ b/src/file/partition.rs @@ -0,0 +1,50 @@ +use anyhow::{Context, Result}; +use std::fs::File; +use std::io::SeekFrom; +use std::io::Seek; +use std::path::Path; + +pub struct PartitionData { + pub num: u32, + pub start: u64, + pub count: u64, +} + +pub fn get_partition_data>(path: P, partition_num: u32) -> Result { + let path = path.as_ref(); + let mut file = File::open(path) + .with_context(|| format!("failed to open image: {}", path.display()))?; + + // Try GPT first (validates CRC32 — more robust than signature check) + if let Ok(gpt) = gptman::GPT::find_from(&mut file) { + let entry = &gpt[partition_num]; + anyhow::ensure!(entry.is_used(), "GPT partition {partition_num} is not used"); + return Ok(PartitionData { + num: partition_num, + start: entry.starting_lba, + count: entry.ending_lba - entry.starting_lba + 1, + }); + } + + // Try MBR — iter() includes logical partitions (5, 6, ...) + file.seek(SeekFrom::Start(0)) + .context("failed to seek to start of image")?; + let mbr = mbrman::MBR::read_from(&mut file, 512) + .context("image is neither valid GPT nor MBR")?; + for (i, p) in mbr.iter() { + if i == partition_num as usize && p.is_used() { + return Ok(PartitionData { + num: partition_num, + start: p.starting_lba as u64, + count: p.sectors as u64, + }); + } + } + anyhow::bail!("partition {partition_num} not found in image") +} + +pub fn is_gpt>(path: P) -> Result { + let mut file = File::open(path.as_ref()) + .context("is_gpt: failed to open image")?; + Ok(gptman::GPT::find_from(&mut file).is_ok()) +} From 1db31b59ae0c9b807cf4c4b119317e5c90a5f151 Mon Sep 17 00:00:00 2001 From: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> Date: Wed, 18 Mar 2026 09:39:21 +0100 Subject: [PATCH 2/4] fix: safety measure for corrupt GPT partitions More a theoretical finding: if for some reason the GPT got corrupt in the WIC file, this change will at least bail out in case the upper sector number in the partition is less than the lower one. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> --- src/file/partition.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/file/partition.rs b/src/file/partition.rs index 24dd2d8..aa50010 100644 --- a/src/file/partition.rs +++ b/src/file/partition.rs @@ -19,10 +19,19 @@ pub fn get_partition_data>(path: P, partition_num: u32) -> Result if let Ok(gpt) = gptman::GPT::find_from(&mut file) { let entry = &gpt[partition_num]; anyhow::ensure!(entry.is_used(), "GPT partition {partition_num} is not used"); + anyhow::ensure!( + entry.ending_lba >= entry.starting_lba, + "GPT partition {partition_num} has invalid LBA range (ending_lba < starting_lba)" + ); + let count = entry + .ending_lba + .checked_sub(entry.starting_lba) + .and_then(|v| v.checked_add(1)) + .context("GPT partition {partition_num} has an LBA range that overflows u64")?; return Ok(PartitionData { num: partition_num, start: entry.starting_lba, - count: entry.ending_lba - entry.starting_lba + 1, + count, }); } From 8199207def0452b72cd04a89ef2e6f90353d34f6 Mon Sep 17 00:00:00 2001 From: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> Date: Wed, 18 Mar 2026 13:41:55 +0100 Subject: [PATCH 3/4] fix(file): address PR review findings - remove unused exec_cmd_with_output! macro left over after fdisk removal - partition.rs: capture GPT parse error and attach as context when MBR fallback also fails, avoiding silent misclassification of I/O errors - partition.rs: replace panicking gpt[n] index with iter().find() for safe bounds-checked GPT partition lookup Signed-off-by: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> --- src/file/functions.rs | 33 ++++++++++----------------- src/file/partition.rs | 52 ++++++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/src/file/functions.rs b/src/file/functions.rs index 0e209cf..4b8cf95 100644 --- a/src/file/functions.rs +++ b/src/file/functions.rs @@ -183,23 +183,6 @@ macro_rules! try_exec_cmd { }; } -macro_rules! exec_cmd_with_output { - ($cmd:expr) => {{ - let res = $cmd - .output() - .context(format!("{}: spawn {:?}", function_name!(), $cmd))?; - - let output = - String::from_utf8(res.stdout).context(format!("{}: get output", function_name!()))?; - - let output = output.trim(); - - debug!("{}: {:?}", function_name!(), $cmd); - - output.to_string() - }}; -} - pub fn copy_to_image(file_copy_params: &[FileCopyToParams], image_file: &Path) -> Result<()> { // we use the folder the image is located in // the caller is responsible to create a /tmp/ directory if needed @@ -382,17 +365,25 @@ pub fn read_file_from_image( fn get_partition_info(image_file: &str, partition: &Partition) -> Result { use crate::file::partition::{get_partition_data, is_gpt}; - let gpt = is_gpt(image_file) - .context("get_partition_info: failed to detect partition table type")?; + let gpt = + is_gpt(image_file).context("get_partition_info: failed to detect partition table type")?; let partition_num: u32 = match partition { Partition::boot => 1, Partition::rootA => 2, Partition::factory => { - if gpt { 4 } else { 5 } + if gpt { + 4 + } else { + 5 + } } Partition::cert => { - if gpt { 5 } else { 6 } + if gpt { + 5 + } else { + 6 + } } }; diff --git a/src/file/partition.rs b/src/file/partition.rs index aa50010..b37210a 100644 --- a/src/file/partition.rs +++ b/src/file/partition.rs @@ -1,7 +1,7 @@ use anyhow::{Context, Result}; use std::fs::File; -use std::io::SeekFrom; use std::io::Seek; +use std::io::SeekFrom; use std::path::Path; pub struct PartitionData { @@ -12,34 +12,37 @@ pub struct PartitionData { pub fn get_partition_data>(path: P, partition_num: u32) -> Result { let path = path.as_ref(); - let mut file = File::open(path) - .with_context(|| format!("failed to open image: {}", path.display()))?; + let mut file = + File::open(path).with_context(|| format!("failed to open image: {}", path.display()))?; - // Try GPT first (validates CRC32 — more robust than signature check) - if let Ok(gpt) = gptman::GPT::find_from(&mut file) { - let entry = &gpt[partition_num]; - anyhow::ensure!(entry.is_used(), "GPT partition {partition_num} is not used"); - anyhow::ensure!( - entry.ending_lba >= entry.starting_lba, - "GPT partition {partition_num} has invalid LBA range (ending_lba < starting_lba)" - ); - let count = entry - .ending_lba - .checked_sub(entry.starting_lba) - .and_then(|v| v.checked_add(1)) - .context("GPT partition {partition_num} has an LBA range that overflows u64")?; - return Ok(PartitionData { - num: partition_num, - start: entry.starting_lba, - count, - }); - } + // Try GPT first (validates CRC32 — more robust than signature check). + // Capture any error so we can attach it as context if MBR also fails. + let gpt_err = match gptman::GPT::find_from(&mut file) { + Ok(gpt) => { + let entry = gpt + .iter() + .find(|(i, _)| *i == partition_num) + .map(|(_, e)| e) + .with_context(|| format!("GPT partition {partition_num} out of range"))?; + anyhow::ensure!(entry.is_used(), "GPT partition {partition_num} is not used"); + anyhow::ensure!( + entry.ending_lba >= entry.starting_lba, + "GPT partition {partition_num} has invalid LBA range (ending_lba < starting_lba)" + ); + return Ok(PartitionData { + num: partition_num, + start: entry.starting_lba, + count: entry.ending_lba - entry.starting_lba + 1, + }); + } + Err(e) => e, + }; // Try MBR — iter() includes logical partitions (5, 6, ...) file.seek(SeekFrom::Start(0)) .context("failed to seek to start of image")?; let mbr = mbrman::MBR::read_from(&mut file, 512) - .context("image is neither valid GPT nor MBR")?; + .with_context(|| format!("image is neither valid GPT nor MBR (GPT error: {gpt_err})"))?; for (i, p) in mbr.iter() { if i == partition_num as usize && p.is_used() { return Ok(PartitionData { @@ -53,7 +56,6 @@ pub fn get_partition_data>(path: P, partition_num: u32) -> Result } pub fn is_gpt>(path: P) -> Result { - let mut file = File::open(path.as_ref()) - .context("is_gpt: failed to open image")?; + let mut file = File::open(path.as_ref()).context("is_gpt: failed to open image")?; Ok(gptman::GPT::find_from(&mut file).is_ok()) } From d9510225d53b52d6f6f8e0b00b28e41b4a58d955 Mon Sep 17 00:00:00 2001 From: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> Date: Wed, 18 Mar 2026 14:01:28 +0100 Subject: [PATCH 4/4] chore: bump version to 0.27.2 Signed-off-by: Harry Waschkeit <44188360+HarryWaschkeit@users.noreply.github.com> --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f3b746..82e37f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2062,7 +2062,7 @@ dependencies = [ [[package]] name = "omnect-cli" -version = "0.27.1" +version = "0.27.2" dependencies = [ "actix-web", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 9b4c11e..23e5655 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ license = "MIT OR Apache-2.0" name = "omnect-cli" readme = "README.md" repository = "https://github.com/omnect/omnect-cli" -version = "0.27.1" +version = "0.27.2" [dependencies] actix-web = "4.11"