diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 60094d96..50671146 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -50,6 +50,19 @@ jobs: timeout-minutes: 10 run: cargo test --locked --release --all --all-features --no-fail-fast -- --nocapture + # The tests attaching loop devices need root, so the run above leaves + # them out. Only the test binary runs as root, which keeps the cached + # build owned by the runner. The MTD ones stay out, as they load the + # mtdram and nandsim kernel modules. + - name: Tests needing root + timeout-minutes: 10 + working-directory: updatehub + run: | + bin=$(cargo test --locked --release --all-features --lib --no-run --message-format=json \ + | sed -n 's/.*"executable":"\([^"]*\)".*/\1/p') + sudo env "PATH=$PATH" "$bin" --ignored --test-threads=1 \ + --skip utils::mtd --skip installer::flash --skip installer::ubifs + - name: Run listener test run: | export UH_LISTENER_TEST=updatehub-statechange.sock diff --git a/flake.nix b/flake.nix index 8ef16f7d..5c4aafaf 100644 --- a/flake.nix +++ b/flake.nix @@ -54,6 +54,7 @@ socat # used by excluded tests + e2fsprogs mtdutils ]; diff --git a/updatehub/src/object/installer/copy.rs b/updatehub/src/object/installer/copy.rs index 01db42b2..549f267f 100644 --- a/updatehub/src/object/installer/copy.rs +++ b/updatehub/src/object/installer/copy.rs @@ -125,14 +125,10 @@ impl Installer for objects::Copy { #[cfg(test)] mod tests { use super::*; - use crate::{object::installer::tests::SERIALIZE, utils::definitions::IdExt}; + use crate::utils::{definitions::IdExt, fs::tests::FakeLoopDevice}; use flate2::{Compression, write::GzEncoder}; use pretty_assertions::assert_eq; - use std::{ - io::{Seek, SeekFrom, Write}, - os::unix::fs::MetadataExt, - path::PathBuf, - }; + use std::{io::Write, os::unix::fs::MetadataExt, path::PathBuf}; use tokio::io::AsyncBufReadExt; const DEFAULT_BYTE: u8 = 0xF; @@ -147,21 +143,9 @@ mod tests { where F: FnMut(&mut objects::Copy), { - // Generate a sparse file for the faked device use - let mut image = tempfile::NamedTempFile::new()?; - image.seek(SeekFrom::Start(1024 * 1024 + FILE_SIZE as u64))?; - image.write_all(&[0])?; - // Setup faked device - let (loopdev, device) = { - // Loop device next_free is not thread safe - let mutex = SERIALIZE.clone(); - let _mutex = mutex.lock().unwrap(); - let loopdev = loopdev::LoopControl::open()?.next_free()?; - let device = loopdev.path().unwrap(); - loopdev.attach_file(image.path())?; - (loopdev, device) - }; + let loop_device = FakeLoopDevice::new(1024 * 1024 + FILE_SIZE as u64 + 1)?; + let device = loop_device.device.clone(); // Format the faked device utils::fs::format(&device, definitions::Filesystem::Ext4, None)?; @@ -258,8 +242,6 @@ mod tests { } } - loopdev.detach()?; - Ok(()) } diff --git a/updatehub/src/object/installer/raw.rs b/updatehub/src/object/installer/raw.rs index 6807ea74..fa5ce2d3 100644 --- a/updatehub/src/object/installer/raw.rs +++ b/updatehub/src/object/installer/raw.rs @@ -13,7 +13,7 @@ use crate::{ }; use pkg_schema::{definitions, objects}; use slog_scope::info; -use std::io::SeekFrom; +use std::{io::SeekFrom, path::Path}; use tokio::{ fs, io::{AsyncRead, AsyncReadExt, AsyncSeekExt}, @@ -24,8 +24,10 @@ impl Installer for objects::Raw { async fn check_requirements(&self, _: &Context) -> Result<()> { info!("'raw' handle checking requirements"); + let access = checked_region(self) + .map_or(Access::Exclusive, |(offset, len)| Access::Region { offset, len }); if let definitions::TargetType::Device(dev) = - self.target_type.valid(Access::Exclusive).log_error_msg("device failed vaidation")? + self.target_type.valid(access).log_error_msg("device failed vaidation")? { utils::fs::ensure_disk_space(dev, self.required_install_size()) .log_error_msg("not enough disk space")?; @@ -43,24 +45,10 @@ impl Installer for objects::Raw { }; let source = context.download_dir.join(self.sha256sum()); let chunk_size = self.chunk_size.0; - let seek = self.seek * chunk_size as u64; - let skip = self.skip.0 * chunk_size as u64; let truncate = self.truncate.0; - 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)) - }; + // The check must stop at the end of what this object writes, otherwise + // a neighbouring object sharing the device answers for it. + let (seek, region_len) = target_region(self); let should_skip_install = super::should_skip_install( self.install_if_different.as_ref(), @@ -77,17 +65,19 @@ impl Installer for objects::Raw { return Ok(()); } - let mut input: Box = { - let mut input = utils::io::timed_buf_reader( - chunk_size, - fs::File::open(source).await.log_error_msg("failed to open source file")?, - ); - input.seek(SeekFrom::Start(skip)).await.log_error_msg("failed to seek source file")?; - match count { - None => Box::new(input), - Some(n) => Box::new(input.take(n)), + // The region check lets a whole disk be written beside the partitions + // it does not reach, even when they are mounted. Only then does the + // region matter, so only then is the write held to it, and the data + // is checked to fit before the device is touched. + let limit = match checked_region(self) { + Some((_, len)) if utils::fs::ensure_not_mounted(device).is_err() => { + ensure_fits(self, &source, len).await?; + Some(len) } + _ => None, }; + + let mut input = open_source(self, &source).await?; let mut target = { let mut target = utils::io::timed_buf_writer( chunk_size, @@ -99,7 +89,7 @@ impl Installer for objects::Raw { .log_error_msg("failed to open target file")?, ); target.seek(SeekFrom::Start(seek)).await.log_error_msg("failed to seek target file")?; - target + utils::io::BoundedWriter::new(target, limit.unwrap_or(u64::MAX)) }; if self.compressed { @@ -116,9 +106,84 @@ impl Installer for objects::Raw { } } +async fn open_source(obj: &objects::Raw, source: &Path) -> Result> { + let mut input = utils::io::timed_buf_reader( + obj.chunk_size.0, + fs::File::open(source).await.log_error_msg("failed to open source file")?, + ); + input + .seek(SeekFrom::Start(obj.skip.0 * obj.chunk_size.0 as u64)) + .await + .log_error_msg("failed to seek source file")?; + + Ok(match count_bytes(obj) { + None => Box::new(input), + Some(n) => Box::new(input.take(n)), + }) +} + +/// A compressed source is uncompressed to measure it, only its metadata +/// telling its size otherwise. +async fn ensure_fits(obj: &objects::Raw, source: &Path, len: u64) -> Result<()> { + let fits = if obj.compressed { + let mut input = open_source(obj, source).await?; + let mut sink = utils::io::BoundedWriter::new(tokio::io::sink(), len); + let res = compress_tools::tokio_support::uncompress_data(&mut input, &mut sink).await; + // Going past the limit fails the uncompression too, that is not a + // corrupted source. + if !sink.overflowed() { + res.log_error_msg("failed to uncompress data")?; + } + !sink.overflowed() + } else { + let available = fs::metadata(source) + .await + .log_error_msg("failed to read source file metadata")? + .len() + .saturating_sub(obj.skip.0 * obj.chunk_size.0 as u64); + count_bytes(obj).map_or(available, |n| n.min(available)) <= len + }; + + if !fits { + return Err(Error::LargerThanDeclared(obj.filename.clone())) + .log_error_msg("object does not fit the region checked for it"); + } + + Ok(()) +} + +fn count_bytes(obj: &objects::Raw) -> Option { + match obj.count { + definitions::Count::All => None, + definitions::Count::Limited(n) => { + Some(u64::try_from(n).unwrap_or(0) * obj.chunk_size.0 as u64) + } + } +} + +/// Offset and length, in bytes, of what `obj` writes to its target. +fn target_region(obj: &objects::Raw) -> (u64, u64) { + let chunk_size = obj.chunk_size.0 as u64; + let len = if obj.compressed { + obj.required_uncompressed_size + } else { + let written = obj.size.saturating_sub(obj.skip.0 * chunk_size); + count_bytes(obj).map_or(written, |n| n.min(written)) + }; + + (obj.seek * chunk_size, len) +} + +/// The region requirements are checked for, when it is known up front. A +/// compressed object not declaring its uncompressed size is not. +fn checked_region(obj: &objects::Raw) -> Option<(u64, u64)> { + (!obj.compressed || obj.required_uncompressed_size != 0).then(|| target_region(obj)) +} + #[cfg(test)] mod tests { use super::*; + use crate::utils::fs::tests::{FakeLoopDevice, LOOP_DEVICE_SIZE}; use flate2::{Compression, write::GzEncoder}; use pretty_assertions::assert_eq; use std::io::{Seek, Write}; @@ -288,6 +353,245 @@ mod tests { (obj, download_dir, source, dest) } + #[test] + fn region_covers_what_is_written() -> Result<()> { + let (obj, ..) = + fake_raw_object(8192, 1024, 1, 4, definitions::Count::Limited(2), false, false)?; + assert_eq!(target_region(&obj), (4096, 2048)); + + let (obj, ..) = fake_raw_object(8192, 1024, 1, 4, definitions::Count::All, false, false)?; + assert_eq!(target_region(&obj), (4096, 7168)); + + let (obj, ..) = + fake_raw_object(8192, 1024, 0, 0, definitions::Count::Limited(16), false, false)?; + assert_eq!(target_region(&obj), (0, 8192)); + + let (mut obj, ..) = + fake_raw_object(8192, 1024, 0, 2, definitions::Count::All, false, true)?; + obj.required_uncompressed_size = 8192; + assert_eq!(target_region(&obj), (2048, 8192)); + + Ok(()) + } + + #[test] + fn compressed_object_without_its_size_has_no_checked_region() -> Result<()> { + let (mut obj, ..) = + fake_raw_object(8192, 1024, 0, 2, definitions::Count::All, false, true)?; + assert_eq!(checked_region(&obj), None); + + obj.required_uncompressed_size = 8192; + assert_eq!(checked_region(&obj), Some((2048, 8192))); + + Ok(()) + } + + fn gzip(data: &[u8]) -> Vec { + let mut e = GzEncoder::new(Vec::new(), Compression::default()); + e.write_all(data).unwrap(); + e.finish().unwrap() + } + + fn raw_object( + download_dir: &std::path::Path, + name: &str, + data: &[u8], + compressed: bool, + seek: u64, + target: &std::path::Path, + ) -> objects::Raw { + let source = if compressed { gzip(data) } else { data.to_vec() }; + std::fs::write(download_dir.join(name), &source).unwrap(); + + objects::Raw { + filename: name.to_string(), + size: source.len() as u64, + sha256sum: name.to_string(), + target_type: definitions::TargetType::Device(target.into()), + + install_if_different: None, + compressed, + required_uncompressed_size: if compressed { data.len() as u64 } else { 0 }, + chunk_size: definitions::ChunkSize(512), + skip: definitions::Skip(0), + seek: seek / 512, + count: definitions::Count::All, + truncate: definitions::Truncate(false), + } + } + + #[tokio::test] + async fn source_fitting_its_region_fits() { + let dir = tempdir().unwrap(); + let data = vec![ORIGINAL_BYTE; 4096]; + for compressed in [false, true] { + let obj = raw_object(dir.path(), "obj", &data, compressed, 0, dir.path()); + let source = dir.path().join("obj"); + ensure_fits(&obj, &source, 4096).await.unwrap(); + } + } + + #[tokio::test] + async fn source_longer_than_its_region_does_not_fit() { + let dir = tempdir().unwrap(); + let data = vec![ORIGINAL_BYTE; 4096]; + for compressed in [false, true] { + let obj = raw_object(dir.path(), "obj", &data, compressed, 0, dir.path()); + let source = dir.path().join("obj"); + let res = ensure_fits(&obj, &source, 4095).await; + assert!( + matches!(res, Err(Error::LargerThanDeclared(_))), + "compressed: {compressed}, got {res:?}" + ); + } + } + + #[tokio::test] + async fn source_fits_once_cut_by_skip_and_count() { + let dir = tempdir().unwrap(); + let mut obj = raw_object(dir.path(), "obj", &[ORIGINAL_BYTE; 4096], false, 0, dir.path()); + let source = dir.path().join("obj"); + obj.skip = definitions::Skip(2); + ensure_fits(&obj, &source, 3072).await.unwrap(); + assert!(ensure_fits(&obj, &source, 3071).await.is_err()); + + obj.count = definitions::Count::Limited(1); + ensure_fits(&obj, &source, 512).await.unwrap(); + } + + #[tokio::test] + async fn corrupted_compressed_source_is_not_reported_as_too_large() { + let dir = tempdir().unwrap(); + let mut obj = raw_object(dir.path(), "obj", &[ORIGINAL_BYTE; 4096], true, 0, dir.path()); + let source = dir.path().join("obj"); + let mut gz = std::fs::read(&source).unwrap(); + gz.truncate(gz.len() / 2); + std::fs::write(&source, gz).unwrap(); + obj.size /= 2; + + let res = ensure_fits(&obj, &source, 4096).await; + assert!(matches!(res, Err(Error::Uncompress(_))), "got {res:?}"); + } + + /// Nothing else being in use, the region is irrelevant and the object is + /// written whole, however much its metadata understates it. + #[tokio::test] + async fn source_longer_than_declared_is_written_whole_when_nothing_is_mounted() { + for compressed in [false, true] { + let (mut obj, download_dir, _source_guard, target_guard, _) = + fake_raw_object(2048, 8, 0, 0, definitions::Count::All, false, compressed).unwrap(); + if compressed { + obj.required_uncompressed_size = 1024; + } else { + obj.size = 1024; + } + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + + obj.check_requirements(&context).await.unwrap(); + obj.install(&context).await.unwrap(); + + let target = std::fs::read(target_guard.path()).unwrap(); + assert!(target.iter().all(|b| *b == ORIGINAL_BYTE), "compressed: {compressed}"); + } + } + + /// Bootloader layout of an am335x: MLO at 128 KiB and U-Boot at 384 KiB of + /// the disk, the rootfs mounted from 1 MiB. + const PARTITION_START: u64 = 1024 * 1024; + const MLO_AT: u64 = 0x2_0000; + const UBOOT_AT: u64 = 0x6_0000; + + fn mounted_disk() -> (FakeLoopDevice, utils::fs::MountGuard) { + let (disk, partition) = + FakeLoopDevice::with_partition(LOOP_DEVICE_SIZE, PARTITION_START).unwrap(); + utils::fs::format(&partition, definitions::Filesystem::Ext4, None).unwrap(); + let guard = utils::fs::mount(&partition, definitions::Filesystem::Ext4, "").unwrap(); + std::fs::write(guard.mount_point().join("canary"), b"alive").unwrap(); + + (disk, guard) + } + + fn read_at(device: &std::path::Path, offset: u64, len: usize) -> Vec { + use std::os::unix::fs::FileExt; + + let mut buf = vec![0; len]; + std::fs::File::open(device).unwrap().read_exact_at(&mut buf, offset).unwrap(); + buf + } + + #[tokio::test] + #[ignore = "needs root to attach a loop device"] + async fn bootloader_installs_beside_a_mounted_partition() { + let (disk, guard) = mounted_disk(); + let download_dir = tempdir().unwrap(); + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + let rootfs_before = read_at(&disk.device, PARTITION_START, 64 * 1024); + + for compressed in [false, true] { + let mlo = vec![0x5a; 0x1_415c]; + let uboot = vec![0xa5; 0x6_2f48]; + for (name, data, at) in [("mlo", &mlo, MLO_AT), ("uboot", &uboot, UBOOT_AT)] { + let obj = raw_object(download_dir.path(), name, data, compressed, at, &disk.device); + obj.check_requirements(&context).await.unwrap(); + obj.install(&context).await.unwrap(); + + assert_eq!(&read_at(&disk.device, at, data.len()), data, "{name}, {compressed}"); + } + } + + assert_eq!(read_at(&disk.device, PARTITION_START, 64 * 1024), rootfs_before); + assert_eq!(std::fs::read(guard.mount_point().join("canary")).unwrap(), b"alive"); + } + + #[tokio::test] + #[ignore = "needs root to attach a loop device"] + async fn object_reaching_a_mounted_partition_is_refused() { + let (disk, _guard) = mounted_disk(); + let download_dir = tempdir().unwrap(); + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + + // Ends a single sector into the partition. + let data = vec![0x5a; usize::try_from(PARTITION_START - UBOOT_AT).unwrap() + 512]; + let obj = raw_object(download_dir.path(), "uboot", &data, false, UBOOT_AT, &disk.device); + let res = obj.check_requirements(&context).await; + + assert!(matches!(res, Err(Error::Utils(utils::Error::DeviceInUse { .. }))), "got {res:?}"); + } + + /// Beside a mounted partition the write is held to the declared region, so + /// an object holding more data must be refused before touching the disk. + #[tokio::test] + #[ignore = "needs root to attach a loop device"] + async fn object_larger_than_declared_beside_a_mounted_partition_is_refused() { + let (disk, _guard) = mounted_disk(); + let download_dir = tempdir().unwrap(); + let context = + Context { download_dir: download_dir.path().to_owned(), ..Context::default() }; + + for compressed in [false, true] { + let data = vec![0x5a; 8192]; + let mut obj = + raw_object(download_dir.path(), "mlo", &data, compressed, MLO_AT, &disk.device); + if compressed { + obj.required_uncompressed_size = 4096; + } else { + obj.size = 4096; + } + obj.check_requirements(&context).await.unwrap(); + let before = read_at(&disk.device, MLO_AT, data.len()); + + let res = obj.install(&context).await; + assert!( + matches!(res, Err(Error::LargerThanDeclared(_))), + "compressed: {compressed}, got {res:?}" + ); + assert_eq!(read_at(&disk.device, MLO_AT, data.len()), before, "{compressed}"); + } + } + /// 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 diff --git a/updatehub/src/object/installer/tarball.rs b/updatehub/src/object/installer/tarball.rs index aa378497..22b749c0 100644 --- a/updatehub/src/object/installer/tarball.rs +++ b/updatehub/src/object/installer/tarball.rs @@ -69,10 +69,9 @@ impl Installer for objects::Tarball { #[cfg(test)] mod tests { use super::*; - use crate::object::installer::tests::SERIALIZE; + use crate::utils::fs::tests::FakeLoopDevice; use pretty_assertions::assert_eq; use std::{ - io::{Seek, SeekFrom, Write}, os::unix::fs::MetadataExt, path::{Path, PathBuf}, }; @@ -83,21 +82,9 @@ mod tests { where F: FnMut(&mut objects::Tarball), { - // Generate a sparse file for the faked device use - let mut image = tempfile::NamedTempFile::new()?; - image.seek(SeekFrom::Start(1024 * 1024 + CONTENT_SIZE as u64))?; - image.write_all(&[0])?; - // Setup faked device - let (loopdev, device) = { - // Loop device next_free is not thread safe - let mutex = SERIALIZE.clone(); - let _mutex = mutex.lock().unwrap(); - let loopdev = loopdev::LoopControl::open()?.next_free()?; - let device = loopdev.path().unwrap(); - loopdev.attach_file(image.path())?; - (loopdev, device) - }; + let loop_device = FakeLoopDevice::new(1024 * 1024 + CONTENT_SIZE as u64 + 1)?; + let device = loop_device.device.clone(); // Format the faked device utils::fs::format(&device, definitions::Filesystem::Ext4, None)?; @@ -147,8 +134,6 @@ mod tests { assert_metadata(&dest.join("tree/branch2/leaf"))?; } - loopdev.detach()?; - Ok(()) } diff --git a/updatehub/src/object/mod.rs b/updatehub/src/object/mod.rs index 46036252..6bf6e887 100644 --- a/updatehub/src/object/mod.rs +++ b/updatehub/src/object/mod.rs @@ -21,6 +21,8 @@ pub enum Error { FwSetEnvNoScriptOption, #[display("unsupported object model")] Unsupported, + #[display("object {_0:?} holds more data than its metadata declares")] + LargerThanDeclared(#[error(not(source))] String), Utils(crate::utils::Error), Firmware(crate::firmware::Error), diff --git a/updatehub/src/utils/definitions.rs b/updatehub/src/utils/definitions.rs index fd7663ab..e1686518 100644 --- a/updatehub/src/utils/definitions.rs +++ b/updatehub/src/utils/definitions.rs @@ -15,6 +15,8 @@ use std::path::PathBuf; pub(crate) enum Access { /// The handler writes the device itself, so nothing else may be using it. Exclusive, + /// The handler writes only `len` bytes at `offset` of the device. + Region { offset: u64, len: u64 }, /// The handler writes through a filesystem it mounts, which is safe to do /// even on a device already mounted elsewhere. ThroughFilesystem, @@ -32,7 +34,8 @@ impl Access { pub(crate) trait TargetTypeExt { /// Checks whether the device is valid to start installation, i.e., /// device exists, use have write permission, and is free of mounted - /// filesystems when the handler asks for [`Access::Exclusive`]. + /// filesystems when the handler asks for [`Access::Exclusive`] or + /// [`Access::Region`]. fn valid(&self, access: Access) -> Result<&Self>; /// Gets device's path for mounting. @@ -51,8 +54,12 @@ impl TargetTypeExt for TargetType { return Err(Error::MissingWritePermission(device)); } - if let Access::Exclusive = access { - fs::ensure_not_mounted(&device)?; + match access { + Access::Exclusive => fs::ensure_not_mounted(&device)?, + Access::Region { offset, len } => { + fs::ensure_region_not_mounted(&device, offset..offset.saturating_add(len))?; + } + Access::ThroughFilesystem => {} } Ok(self) diff --git a/updatehub/src/utils/fs.rs b/updatehub/src/utils/fs.rs index a76daf83..36202f1d 100644 --- a/updatehub/src/utils/fs.rs +++ b/updatehub/src/utils/fs.rs @@ -12,6 +12,7 @@ use slog_scope::trace; use std::{ collections::HashSet, io::{self, Seek}, + ops::Range, os::unix::fs::FileTypeExt, path::{Path, PathBuf}, }; @@ -74,6 +75,21 @@ type DeviceId = (u64, u64); pub(crate) fn ensure_not_mounted(target: &Path) -> Result<()> { trace!("checking whether {:?} is in use", target); + ensure_devices_not_mounted(target, related_block_devices) +} + +/// Like [`ensure_not_mounted`], but ignores the partitions of a whole disk +/// lying outside `region`. +pub(crate) fn ensure_region_not_mounted(target: &Path, region: Range) -> Result<()> { + trace!("checking whether {:?} is in use within {:?}", target, region); + + ensure_devices_not_mounted(target, |device| region_block_devices(device, ®ion)) +} + +fn ensure_devices_not_mounted( + target: &Path, + related: impl Fn(DeviceId) -> HashSet, +) -> Result<()> { let metadata = match std::fs::metadata(target) { Ok(metadata) => metadata, // Nothing can be mounted on a device which is not there. Complaining @@ -87,10 +103,10 @@ pub(crate) fn ensure_not_mounted(target: &Path) -> Result<()> { let sources = mtd::mount_sources_for(target).into_iter().collect::>(); let mut devices = HashSet::new(); - devices.extend(block_device_id(&metadata).into_iter().flat_map(related_block_devices)); + devices.extend(block_device_id(&metadata).into_iter().flat_map(&related)); for source in sources.iter().filter(|source| source.starts_with('/')) { if let Ok(metadata) = std::fs::metadata(source) { - devices.extend(block_device_id(&metadata).into_iter().flat_map(related_block_devices)); + devices.extend(block_device_id(&metadata).into_iter().flat_map(&related)); } } @@ -180,6 +196,47 @@ fn related_block_devices(device: DeviceId) -> HashSet { found } +/// Like [`related_block_devices`], without the partitions lying outside +/// `region`. +fn region_block_devices(device: DeviceId, region: &Range) -> HashSet { + let sysfs = PathBuf::from(format!("/sys/dev/block/{}:{}", device.0, device.1)); + let Some(sysfs) = sysfs.canonicalize().ok().filter(|sysfs| !sysfs.join("partition").exists()) + else { + return related_block_devices(device); + }; + + let mut found = HashSet::from([device]); + found.extend( + device_ids_in(&sysfs.join("holders"), |_| true).into_iter().flat_map(related_block_devices), + ); + // Unreadable bounds count as overlapping. + found.extend( + device_ids_in(&sysfs, |path| { + path.join("partition").exists() + && partition_range(path).is_none_or(|partition| overlaps(&partition, region)) + }) + .into_iter() + .flat_map(related_block_devices), + ); + + found +} + +/// sysfs reports `start` and `size` in 512 byte sectors. +fn partition_range(path: &Path) -> Option> { + let sectors = |name: &str| -> Option { + std::fs::read_to_string(path.join(name)).ok()?.trim().parse().ok() + }; + let start = sectors("start")?.checked_mul(512)?; + let end = start.checked_add(sectors("size")?.checked_mul(512)?)?; + + Some(start..end) +} + +fn overlaps(a: &Range, b: &Range) -> bool { + a.start.max(b.start) < a.end.min(b.end) +} + /// Device numbers of the sysfs entries under `dir` which satisfy `wanted`. fn device_ids_in(dir: &Path, wanted: impl Fn(&Path) -> bool) -> Vec { let Ok(entries) = std::fs::read_dir(dir) else { @@ -341,7 +398,7 @@ pub(crate) fn chown(path: &Path, uid: Option<&Uid>, gid: Option<&Gid>) -> Result } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::{search_path::SearchPathGuard, *}; use crate::object::installer::tests::SERIALIZE; use pretty_assertions::assert_eq; @@ -410,16 +467,17 @@ mod tests { // Big enough to tell a loop device apart from the devtmpfs on /dev, small // enough to stay sparse on any builder. - const LOOP_DEVICE_SIZE: u64 = 64 * 1024 * 1024; + pub(crate) const LOOP_DEVICE_SIZE: u64 = 64 * 1024 * 1024; - struct FakeLoopDevice { + /// Detached when dropped, so a failing test does not leak it. + pub(crate) struct FakeLoopDevice { loopdev: loopdev::LoopDevice, - device: PathBuf, + pub(crate) device: PathBuf, _backing_file: tempfile::NamedTempFile, } impl FakeLoopDevice { - fn new(size: u64) -> Result { + pub(crate) fn new(size: u64) -> Result { let backing_file = tempfile::NamedTempFile::new()?; backing_file.as_file().set_len(size)?; @@ -432,6 +490,45 @@ mod tests { Ok(FakeLoopDevice { loopdev, device, _backing_file: backing_file }) } + + /// A loop device holding a single partition from `start` to its end, + /// returned along with the partition's own device node. + pub(crate) fn with_partition(size: u64, start: u64) -> Result<(FakeLoopDevice, PathBuf)> { + let backing_file = tempfile::NamedTempFile::new()?; + backing_file.as_file().set_len(size)?; + write_mbr(backing_file.as_file(), start / 512, (size - start) / 512)?; + + let mutex = SERIALIZE.clone(); + let _mutex = mutex.lock().unwrap(); + let loopdev = loopdev::LoopControl::open()?.next_free()?; + let device = loopdev.path().unwrap(); + loopdev.with().part_scan(true).attach(backing_file.path())?; + let loop_device = FakeLoopDevice { loopdev, device, _backing_file: backing_file }; + + // The partition node shows up asynchronously after the scan. + let partition = PathBuf::from(format!("{}p1", loop_device.device.display())); + for _ in 0..50 { + if partition.exists() { + return Ok((loop_device, partition)); + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + + Err(io::Error::new(io::ErrorKind::NotFound, format!("{partition:?} did not show up")) + .into()) + } + } + + /// Writes an MBR with a single Linux partition of `sectors` from `start`. + fn write_mbr(file: &std::fs::File, start: u64, sectors: u64) -> io::Result<()> { + use std::os::unix::fs::FileExt; + + let mut entry = [0u8; 16]; + entry[4] = 0x83; + entry[8..12].copy_from_slice(&u32::try_from(start).unwrap().to_le_bytes()); + entry[12..16].copy_from_slice(&u32::try_from(sectors).unwrap().to_le_bytes()); + file.write_all_at(&entry, 446)?; + file.write_all_at(&[0x55, 0xaa], 510) } impl Drop for FakeLoopDevice { @@ -514,6 +611,38 @@ mod tests { assert!(parse_mount_entry("26 25 bogus / /boot rw - ext4 /dev/sda2 rw").is_none()); } + #[test] + fn regions_overlap_only_when_sharing_a_byte() { + let rootfs = 0x10_0000..0x6a50_0000; + assert!(!overlaps(&(0x2_0000..0x3_415c), &rootfs)); + assert!(!overlaps(&(0x6_0000..0xc_2f48), &rootfs)); + + assert!(!overlaps(&(0x8_0000..0x10_0000), &rootfs)); + assert!(overlaps(&(0x8_0000..0x10_0001), &rootfs)); + assert!(overlaps(&(0x20_0000..0x30_0000), &rootfs)); + assert!(overlaps(&(0..u64::MAX), &rootfs)); + assert!(!overlaps(&(0x20_0000..0x20_0000), &rootfs)); + } + + #[test] + fn partition_range_is_read_in_sectors() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("start"), "2048\n").unwrap(); + std::fs::write(dir.path().join("size"), "3481600\n").unwrap(); + + assert_eq!(partition_range(dir.path()), Some(0x10_0000..0x6a50_0000)); + } + + #[test] + fn partition_range_needs_both_bounds() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("start"), "2048\n").unwrap(); + assert_eq!(partition_range(dir.path()), None); + + std::fs::write(dir.path().join("size"), "bogus\n").unwrap(); + assert_eq!(partition_range(dir.path()), None); + } + #[test] fn regular_file_is_not_in_use() { let file = tempfile::NamedTempFile::new().unwrap(); @@ -548,4 +677,30 @@ mod tests { "a mounted device must be rejected, got {in_use:?}" ); } + + #[test] + #[ignore = "needs root to attach a loop device"] + fn region_check_ignores_partitions_outside_the_region() { + const PARTITION_START: u64 = 1024 * 1024; + let (loop_device, partition) = + FakeLoopDevice::with_partition(LOOP_DEVICE_SIZE, PARTITION_START).unwrap(); + format(&partition, Filesystem::Ext4, None).unwrap(); + let guard = mount(&partition, Filesystem::Ext4, "").unwrap(); + + let gap = ensure_region_not_mounted(&loop_device.device, 0x2_0000..PARTITION_START); + let overlapping = + ensure_region_not_mounted(&loop_device.device, 0x2_0000..PARTITION_START + 1); + let whole_disk = ensure_not_mounted(&loop_device.device); + let on_the_partition = ensure_region_not_mounted(&partition, 0..512); + // Unmount before asserting so the device is always released. + drop(guard); + + assert!(gap.is_ok(), "the gap ahead of the partition should be free, got {gap:?}"); + for in_use in [overlapping, whole_disk, on_the_partition] { + assert!( + matches!(in_use, Err(Error::DeviceInUse { .. })), + "a write reaching the mounted partition must be rejected, got {in_use:?}" + ); + } + } } diff --git a/updatehub/src/utils/io.rs b/updatehub/src/utils/io.rs index e7670e18..b4bb91ba 100644 --- a/updatehub/src/utils/io.rs +++ b/updatehub/src/utils/io.rs @@ -3,7 +3,12 @@ // SPDX-License-Identifier: Apache-2.0 use slog_scope::trace; -use std::{pin::Pin, time::Duration}; +use std::{ + io, + pin::Pin, + task::{Context, Poll, ready}, + time::Duration, +}; use tokio::io::{AsyncRead, AsyncWrite, BufReader, BufWriter}; use tokio_io_timeout::{TimeoutReader, TimeoutWriter}; @@ -32,3 +37,78 @@ where w.set_timeout(Some(Duration::from_secs(5))); Box::pin(BufWriter::with_capacity(chunk_size, w)) } + +/// Fails a write going past `limit` bytes, so an object cannot reach beyond +/// the region its requirements were checked for. +pub(crate) struct BoundedWriter { + inner: W, + remaining: u64, + overflowed: bool, +} + +impl BoundedWriter { + pub(crate) fn new(inner: W, limit: u64) -> Self { + BoundedWriter { inner, remaining: limit, overflowed: false } + } + + pub(crate) fn overflowed(&self) -> bool { + self.overflowed + } +} + +impl AsyncWrite for BoundedWriter { + fn poll_write( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + if self.remaining == 0 && !buf.is_empty() { + self.overflowed = true; + return Poll::Ready(Err(io::Error::other( + "data goes past the region checked for the object", + ))); + } + + let len = buf.len().min(usize::try_from(self.remaining).unwrap_or(usize::MAX)); + let written = ready!(Pin::new(&mut self.inner).poll_write(cx, &buf[..len]))?; + self.remaining -= written as u64; + + Poll::Ready(Ok(written)) + } + + fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_flush(cx) + } + + fn poll_shutdown(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.inner).poll_shutdown(cx) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::AsyncWriteExt; + + #[tokio::test] + async fn bounded_writer_takes_up_to_its_limit() { + let mut out = Vec::new(); + let mut w = BoundedWriter::new(&mut out, 4); + w.write_all(b"abcd").await.unwrap(); + w.flush().await.unwrap(); + assert!(!w.overflowed()); + + assert_eq!(out, b"abcd"); + } + + #[tokio::test] + async fn bounded_writer_refuses_what_goes_past_its_limit() { + let mut out = Vec::new(); + let mut w = BoundedWriter::new(&mut out, 4); + let res = w.write_all(b"abcdef").await; + + assert!(res.is_err(), "writing past the limit must fail"); + assert!(w.overflowed()); + assert_eq!(out, b"abcd"); + } +}