From 234e3a2dd1af414cc9f87143dea1c82041c54b26 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 10 Sep 2026 11:07:23 -0300 Subject: [PATCH 1/2] fix: stop the raw install-if-different check at the end of the object The check handed `should_skip_install` a reader over the whole target device whenever `count` was `-1`, so it kept scanning well past the bytes the object owns. Objects sharing a device at different offsets then answer for each other. An i.MX6ULL package hit this on a downgrade. It writes SPL at `seek` 1 and U-Boot proper at `seek` 64 of the same `/dev/mtdblock1`, both guarded by the same U-Boot version, and objects install largest first: U-Boot proper landed on the device first, and the SPL check then read straight through its own region into the U-Boot image just written, found the version it was looking for, and skipped the SPL. The device kept the previous SPL while every other object downgraded. Bound the reader to the bytes the install writes. For a compressed object that is the uncompressed size, since `skip` and `count` measure the source rather than the region they land in; otherwise it is `size` minus `skip`, narrowed by `count` when it is limited. Turning `count` into a byte length up front also lets the write path share it, and drops both the clone it needed and the `AsyncReadSeek` boxing. --- updatehub/src/object/installer/raw.rs | 132 ++++++++++++++++++++++---- 1 file changed, 114 insertions(+), 18 deletions(-) diff --git a/updatehub/src/object/installer/raw.rs b/updatehub/src/object/installer/raw.rs index af5c5ffb..6807ea74 100644 --- a/updatehub/src/object/installer/raw.rs +++ b/updatehub/src/object/installer/raw.rs @@ -16,7 +16,7 @@ use slog_scope::info; use std::io::SeekFrom; use tokio::{ fs, - io::{AsyncRead, AsyncReadExt, AsyncSeek, AsyncSeekExt}, + io::{AsyncRead, AsyncReadExt, AsyncSeekExt}, }; use tokio_take_seek::AsyncTakeSeekExt; @@ -46,26 +46,30 @@ impl Installer for objects::Raw { let seek = self.seek * chunk_size as u64; let skip = self.skip.0 * chunk_size as u64; let truncate = self.truncate.0; - let count = self.count.clone(); + let count = match self.count { + definitions::Count::All => None, + definitions::Count::Limited(n) => { + Some(u64::try_from(n).unwrap_or(0) * chunk_size as u64) + } + }; + + // How much of the target this object writes. The check must stop there, + // otherwise a neighbouring object sharing the device answers for it. + let region_len = if self.compressed { + self.required_uncompressed_size + } else { + let written = self.size.saturating_sub(skip); + count.map_or(written, |n| n.min(written)) + }; let should_skip_install = super::should_skip_install( self.install_if_different.as_ref(), &self.sha256sum, async { - trait AsyncReadSeek: AsyncRead + AsyncSeek + Unpin {} - impl AsyncReadSeek for R {} - let h = fs::OpenOptions::new().read(true).open(device).await?; let mut h = utils::io::timed_buf_reader(chunk_size, h); h.seek(SeekFrom::Start(seek)).await?; - let h: Box = match &count { - definitions::Count::All => Box::new(h), - definitions::Count::Limited(n) => { - let count = u64::try_from(*n).unwrap_or(0); - Box::new(h.take_with_seek(count * chunk_size as u64)) - } - }; - Ok(h) + Ok(h.take_with_seek(region_len)) }, ) .await?; @@ -80,11 +84,8 @@ impl Installer for objects::Raw { ); input.seek(SeekFrom::Start(skip)).await.log_error_msg("failed to seek source file")?; match count { - definitions::Count::All => Box::new(input), - definitions::Count::Limited(n) => { - let count = u64::try_from(n).unwrap_or(0); - Box::new(input.take(count * chunk_size as u64)) - } + None => Box::new(input), + Some(n) => Box::new(input.take(n)), } }; let mut target = { @@ -229,6 +230,101 @@ mod tests { Ok(()) } + const SHARED_CHUNK_SIZE: usize = 1024; + const SHARED_OBJECT_LEN: usize = 4 * SHARED_CHUNK_SIZE; + const SHARED_OBJECT_AT: usize = SHARED_CHUNK_SIZE; + const SHARED_NEIGHBOUR_AT: usize = 64 * SHARED_CHUNK_SIZE; + + /// Lays out a device shared by two raw objects, the one under test at + /// `SHARED_OBJECT_AT` and a neighbour at `SHARED_NEIGHBOUR_AT`, each + /// holding a U-Boot banner of the given version. + fn fake_shared_device( + rule_version: &str, + object_version: Option<&str>, + neighbour_version: &str, + ) -> (objects::Raw, TempDir, NamedTempFile, NamedTempFile) { + let banner = |v: &str| format!("U-Boot {v} (Jul 18 2023 - 18:41:01 +0000)\0").into_bytes(); + + let download_dir = tempdir().unwrap(); + + let mut source = NamedTempFile::new_in(download_dir.path()).unwrap(); + source + .write_all(&std::iter::repeat_n(ORIGINAL_BYTE, SHARED_OBJECT_LEN).collect::>()) + .unwrap(); + source.flush().unwrap(); + + let mut dest = NamedTempFile::new_in(download_dir.path()).unwrap(); + dest.write_all( + &std::iter::repeat_n(DEFAULT_BYTE, 2 * SHARED_NEIGHBOUR_AT).collect::>(), + ) + .unwrap(); + if let Some(v) = object_version { + dest.seek(SeekFrom::Start(SHARED_OBJECT_AT as u64)).unwrap(); + dest.write_all(&banner(v)).unwrap(); + } + dest.seek(SeekFrom::Start(SHARED_NEIGHBOUR_AT as u64)).unwrap(); + dest.write_all(&banner(neighbour_version)).unwrap(); + dest.flush().unwrap(); + + let obj = objects::Raw { + filename: String::new(), + size: SHARED_OBJECT_LEN as u64, + sha256sum: source.path().to_string_lossy().to_string(), + target_type: definitions::TargetType::Device(dest.path().into()), + + install_if_different: Some(definitions::InstallIfDifferent::KnownPattern { + version: rule_version.to_string(), + pattern: definitions::install_if_different::KnownPatternKind::UBoot, + }), + compressed: false, + required_uncompressed_size: 0, + chunk_size: definitions::ChunkSize(SHARED_CHUNK_SIZE), + skip: definitions::Skip(0), + seek: (SHARED_OBJECT_AT / SHARED_CHUNK_SIZE) as u64, + count: definitions::Count::All, + truncate: definitions::Truncate(false), + }; + + (obj, download_dir, source, dest) + } + + /// The check must stop at the end of the object it guards. Objects install + /// largest first, so the neighbour is already carrying the new version by + /// the time this one is checked, and reading into it skips an object that + /// did change. + #[tokio::test] + async fn install_if_different_stops_at_the_end_of_the_object() { + let (obj, download_dir, _source_guard, target_guard) = + fake_shared_device("2020.01", None, "2020.01"); + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + + obj.install(&context).await.unwrap(); + + let written = std::fs::read(target_guard.path()).unwrap(); + assert_eq!( + written[SHARED_OBJECT_AT..SHARED_OBJECT_AT + SHARED_OBJECT_LEN], + std::iter::repeat_n(ORIGINAL_BYTE, SHARED_OBJECT_LEN).collect::>(), + "the neighbour's version answered for this object and skipped it" + ); + } + + /// The counterpart: a matching version inside the object's own region still + /// skips the installation. + #[tokio::test] + async fn install_if_different_matches_inside_the_object() { + let (obj, download_dir, _source_guard, target_guard) = + fake_shared_device("2020.01", Some("2020.01"), "2019.04"); + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + + let before = std::fs::read(target_guard.path()).unwrap(); + obj.install(&context).await.unwrap(); + let after = std::fs::read(target_guard.path()).unwrap(); + + assert_eq!(before, after, "object with an unchanged version should not be installed"); + } + #[tokio::test] async fn raw_full_copy_compressed() { let size = 2048; From dbb2990732e6dc5f2af4c471820b8e488d50d37d Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 10 Sep 2026 11:22:42 -0300 Subject: [PATCH 2/2] build(deps): require find-binary-version 0.5.2 0.5.0 cannot read a version banner that lands across the boundary of one of its 0x200 reads, and reports no version at all when that happens. The check guarding an object then reads as "the target differs" every time, so an object that never changed is written on every update, and the comparison the object asked for never actually runs. The i.MX6ULL builds this showed up on sat on both sides of that: one pair of binaries kept their banners clear of a boundary and compared fine, the other pair straddled one and never compared at all. 0.5.1 carries that fix, and 0.5.2 stops the custom-pattern search from reading the whole source into memory before matching, which for a rule pointed at a partition meant holding the image alongside the device it came from. Naming the version rather than any 0.5 keeps a fresh resolve from picking up either of the older behaviours. --- Cargo.lock | 4 ++-- updatehub/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f629f35e..55971d04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -955,9 +955,9 @@ dependencies = [ [[package]] name = "find-binary-version" -version = "0.5.0" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83d19b93dc99c358a96dfb3f82d233d97764031795031fb390e9575907f77b35" +checksum = "7fb400f566b8ed02db7d3e14489fabaf0fabbef349bd9cfe8bfd17bb7fc625d1" dependencies = [ "async-trait", "compress-tools 0.14.3", diff --git a/updatehub/Cargo.toml b/updatehub/Cargo.toml index 2fa828bb..6b056f8a 100644 --- a/updatehub/Cargo.toml +++ b/updatehub/Cargo.toml @@ -69,7 +69,7 @@ derive_more = { version = "2", default-features = false, features = [ "from", ] } easy_process = "0.2" -find-binary-version = "0.5" +find-binary-version = "0.5.2" futures-util = { version = "0.3", default-features = false } logging_content = "0.1" mockito = { version = "1", optional = true }