diff --git a/updatehub/src/object/installer/copy.rs b/updatehub/src/object/installer/copy.rs index 32b5ed70..b004b082 100644 --- a/updatehub/src/object/installer/copy.rs +++ b/updatehub/src/object/installer/copy.rs @@ -5,7 +5,11 @@ use super::{Context, Error, Result}; use crate::{ object::{Info, Installer}, - utils::{self, definitions::TargetTypeExt, log::LogContent}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + log::LogContent, + }, }; use pkg_schema::{definitions, objects}; use slog_scope::info; @@ -20,8 +24,10 @@ impl Installer for objects::Copy { async fn check_requirements(&self, _: &Context) -> Result<()> { info!("'copy' handle checking requirements"); - if let definitions::TargetType::Device(_) = - self.target_type.valid().log_error_msg("device failed vaidation")? + if let definitions::TargetType::Device(_) = self + .target_type + .valid(Access::for_format(self.target_format.should_format)) + .log_error_msg("device failed vaidation")? { utils::fs::ensure_disk_space( &self.target_type.get_target()?, diff --git a/updatehub/src/object/installer/flash.rs b/updatehub/src/object/installer/flash.rs index 2b9d99c6..344387a7 100644 --- a/updatehub/src/object/installer/flash.rs +++ b/updatehub/src/object/installer/flash.rs @@ -5,7 +5,10 @@ use super::{Context, Error, Result}; use crate::{ object::{Info, Installer}, - utils::{self, definitions::TargetTypeExt}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + }, }; use pkg_schema::{definitions, objects}; @@ -21,7 +24,7 @@ impl Installer for objects::Flash { match self.target { definitions::TargetType::Device(_) | definitions::TargetType::MTDName(_) => { - self.target.valid()?; + self.target.valid(Access::Exclusive)?; utils::fs::ensure_disk_space( &self.target.get_target()?, self.required_install_size(), diff --git a/updatehub/src/object/installer/imxkobs.rs b/updatehub/src/object/installer/imxkobs.rs index 721c4e21..ced06bf1 100644 --- a/updatehub/src/object/installer/imxkobs.rs +++ b/updatehub/src/object/installer/imxkobs.rs @@ -11,12 +11,26 @@ use pkg_schema::objects; use slog_scope::info; use std::{fmt::Write as _, path::PathBuf}; +/// Device `kobs-ng` writes to when the object does not name one. +const DEFAULT_CHIP_0_DEVICE_PATH: &str = "/dev/mtd0"; + +/// Device `kobs-ng` writes the bootstream to, which it falls back to on its own +/// when the object names none. +fn chip_0_path(obj: &objects::Imxkobs) -> PathBuf { + obj.chip_0_device_path.clone().unwrap_or_else(|| PathBuf::from(DEFAULT_CHIP_0_DEVICE_PATH)) +} + #[async_trait::async_trait(?Send)] impl Installer for objects::Imxkobs { async fn check_requirements(&self, _: &Context) -> Result<()> { info!("'imxkobs' handle checking requirements"); utils::fs::is_executable_in_path("kobs-ng").log_error_msg("kobs-ng not in PATH")?; + for chip in [Some(chip_0_path(self)), self.chip_1_device_path.clone()].into_iter().flatten() + { + utils::fs::ensure_not_mounted(&chip).log_error_msg("target device is in use")?; + } + Ok(()) } @@ -25,8 +39,7 @@ impl Installer for objects::Imxkobs { let should_skip_install = super::should_skip_install(&self.install_if_different, &self.sha256sum, async { - let path = - self.chip_0_device_path.clone().unwrap_or_else(|| PathBuf::from("/dev/mtd0")); + let path = chip_0_path(self); let f = path.file_name().ok_or(Error::InvalidPath)?; let mut file_name = f.to_os_string(); file_name.push("ro"); @@ -85,8 +98,10 @@ mod tests { install_if_different: None, padding_1k: true, search_exponent: 2, - chip_0_device_path: Some(PathBuf::from("/dev/sda1")), - chip_1_device_path: Some(PathBuf::from("/dev/sda2")), + // Inexistent on purpose, a real device could be in use on the host + // running the tests and the requirements check would reject it. + chip_0_device_path: Some(PathBuf::from("/dev/updatehub-chip-0")), + chip_1_device_path: Some(PathBuf::from("/dev/updatehub-chip-1")), } } diff --git a/updatehub/src/object/installer/mod.rs b/updatehub/src/object/installer/mod.rs index 67e408d6..d00e4f8f 100644 --- a/updatehub/src/object/installer/mod.rs +++ b/updatehub/src/object/installer/mod.rs @@ -129,7 +129,7 @@ where } #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use lazy_static::lazy_static; use std::{ diff --git a/updatehub/src/object/installer/raw.rs b/updatehub/src/object/installer/raw.rs index 937200cc..1a631a55 100644 --- a/updatehub/src/object/installer/raw.rs +++ b/updatehub/src/object/installer/raw.rs @@ -5,7 +5,11 @@ use super::{Context, Error, Result}; use crate::{ object::{Info, Installer}, - utils::{self, definitions::TargetTypeExt, log::LogContent}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + log::LogContent, + }, }; use pkg_schema::{definitions, objects}; use slog_scope::info; @@ -22,7 +26,7 @@ impl Installer for objects::Raw { info!("'raw' handle checking requirements"); if let definitions::TargetType::Device(dev) = - self.target_type.valid().log_error_msg("device failed vaidation")? + self.target_type.valid(Access::Exclusive).log_error_msg("device failed vaidation")? { utils::fs::ensure_disk_space(dev, self.required_install_size()) .log_error_msg("not enough disk space")?; diff --git a/updatehub/src/object/installer/raw_delta.rs b/updatehub/src/object/installer/raw_delta.rs index f3ef6ed8..20e2fb32 100644 --- a/updatehub/src/object/installer/raw_delta.rs +++ b/updatehub/src/object/installer/raw_delta.rs @@ -5,7 +5,12 @@ use super::{Context, Error, Result}; use crate::{ object::Installer, - utils::{self, definitions::TargetTypeExt, delta, log::LogContent}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + delta, + log::LogContent, + }, }; use pkg_schema::{definitions, objects}; @@ -17,7 +22,7 @@ impl Installer for objects::RawDelta { info!("'raw-delta' handle checking requirements"); if let definitions::TargetType::Device(dev) = - self.target.valid().log_error_msg("device failed validation")? + self.target.valid(Access::Exclusive).log_error_msg("device failed validation")? { let seed = get_seed_path(self, context); let required_size = delta::get_required_size(&seed, dev) diff --git a/updatehub/src/object/installer/tarball.rs b/updatehub/src/object/installer/tarball.rs index b5d1b3ea..9d67090d 100644 --- a/updatehub/src/object/installer/tarball.rs +++ b/updatehub/src/object/installer/tarball.rs @@ -5,7 +5,11 @@ use super::{Context, Result}; use crate::{ object::{Info, Installer}, - utils::{self, definitions::TargetTypeExt, log::LogContent}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + log::LogContent, + }, }; use pkg_schema::{definitions, objects}; use slog_scope::info; @@ -24,7 +28,9 @@ impl Installer for objects::Tarball { self.required_install_size(), ) .log_error_msg("not enough disk space")?; - self.target.valid().log_error_msg("device failed validation")?; + self.target + .valid(Access::for_format(self.target_format.should_format)) + .log_error_msg("device failed validation")?; Ok(()) } } diff --git a/updatehub/src/object/installer/ubifs.rs b/updatehub/src/object/installer/ubifs.rs index 879dcc76..3c48a8f8 100644 --- a/updatehub/src/object/installer/ubifs.rs +++ b/updatehub/src/object/installer/ubifs.rs @@ -5,7 +5,11 @@ use super::{Context, Error, Result}; use crate::{ object::{Info, Installer}, - utils::{self, definitions::TargetTypeExt, log::LogContent}, + utils::{ + self, + definitions::{Access, TargetTypeExt}, + log::LogContent, + }, }; use pkg_schema::{definitions, objects}; use slog_scope::info; @@ -20,7 +24,7 @@ impl Installer for objects::Ubifs { utils::fs::is_executable_in_path("ubinfo").log_error_msg("ubinfo not on PATH")?; if let definitions::TargetType::UBIVolume(_) = - self.target.valid().log_error_msg("device failed validation")? + self.target.valid(Access::Exclusive).log_error_msg("device failed validation")? { utils::fs::ensure_disk_space(&self.target.get_target()?, self.required_install_size()) .log_error_msg("not enough disk space")?; diff --git a/updatehub/src/utils/definitions.rs b/updatehub/src/utils/definitions.rs index b6aa72ca..13597bdc 100644 --- a/updatehub/src/utils/definitions.rs +++ b/updatehub/src/utils/definitions.rs @@ -3,25 +3,44 @@ // SPDX-License-Identifier: Apache-2.0 use super::{Error, Result}; -use crate::utils::mtd; +use crate::utils::{fs, mtd}; use pkg_schema::definitions::{ TargetType, target_permissions::{Gid, Uid}, }; use std::path::PathBuf; +/// How a handler reaches its target, which decides whether a filesystem the +/// system already has mounted on it is acceptable. +pub(crate) enum Access { + /// The handler writes the device itself, so nothing else may be using it. + Exclusive, + /// The handler writes through a filesystem it mounts, which is safe to do + /// even on a device already mounted elsewhere. + ThroughFilesystem, +} + +impl Access { + /// Formatting rewrites the whole filesystem, so it takes the device for + /// itself even on a handler otherwise going through a mount. + pub(crate) fn for_format(should_format: bool) -> Access { + if should_format { Access::Exclusive } else { Access::ThroughFilesystem } + } +} + /// Utility functions for [TargetType](pkg_schema::definitions::TargetType) pub(crate) trait TargetTypeExt { /// Checks whether the device is valid to start installation, i.e., - /// device exists, use have write permission. - fn valid(&self) -> Result<&Self>; + /// device exists, use have write permission, and is free of mounted + /// filesystems when the handler asks for [Access::Exclusive]. + fn valid(&self, access: Access) -> Result<&Self>; /// Gets device's path for mounting. fn get_target(&self) -> Result; } impl TargetTypeExt for TargetType { - fn valid(&self) -> Result<&Self> { + fn valid(&self, access: Access) -> Result<&Self> { let device = self.get_target()?; if !device.exists() { @@ -32,6 +51,10 @@ impl TargetTypeExt for TargetType { return Err(Error::MissingWritePermission(device)); } + if let Access::Exclusive = access { + fs::ensure_not_mounted(&device)?; + } + Ok(self) } diff --git a/updatehub/src/utils/fs.rs b/updatehub/src/utils/fs.rs index 887e9323..87fd813a 100644 --- a/updatehub/src/utils/fs.rs +++ b/updatehub/src/utils/fs.rs @@ -3,13 +3,17 @@ // SPDX-License-Identifier: Apache-2.0 use super::{Error, Result}; -use crate::utils::definitions::IdExt; +use crate::utils::{definitions::IdExt, mtd}; use pkg_schema::definitions::{ Filesystem, target_permissions::{Gid, Uid}, }; use slog_scope::trace; -use std::{io, path::Path}; +use std::{ + collections::HashSet, + io, + path::{Path, PathBuf}, +}; use sys_mount::{Mount, Unmount, UnmountDrop}; pub(crate) struct MountGuard { @@ -37,6 +41,145 @@ pub(crate) fn ensure_disk_space(target: &Path, required: u64) -> Result<()> { Ok(()) } +/// Block device number, as `(major, minor)`. +type DeviceId = (u64, u64); + +/// Ensures no mounted filesystem lives on `target`, so an installation writing +/// to it cannot corrupt data the running system still believes it owns. +/// +/// Besides `target` itself this covers the partitions it contains, the disk it +/// is a partition of and whatever is stacked on top of it, as damaging any of +/// those damages the others. +pub(crate) fn ensure_not_mounted(target: &Path) -> Result<()> { + trace!("checking whether {:?} is in use", target); + + let metadata = match std::fs::metadata(target) { + Ok(metadata) => metadata, + // Nothing can be mounted on a device which is not there. Complaining + // about it is left to the existence check. + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e.into()), + }; + + // MTD and UBI offer no block device to compare against, they are named in + // `/proc/self/mountinfo` instead. + 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)); + 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)); + } + } + + let mountinfo = std::fs::read_to_string("/proc/self/mountinfo")?; + if let Some(entry) = mountinfo + .lines() + .filter_map(parse_mount_entry) + .find(|entry| devices.contains(&entry.device) || sources.contains(&entry.source)) + { + return Err(Error::DeviceInUse { + device: target.to_owned(), + mount_point: entry.mount_point, + }); + } + + Ok(()) +} + +fn block_device_id(metadata: &std::fs::Metadata) -> Option { + use std::os::unix::fs::{FileTypeExt, MetadataExt}; + + metadata + .file_type() + .is_block_device() + .then(|| (nix::sys::stat::major(metadata.rdev()), nix::sys::stat::minor(metadata.rdev()))) +} + +/// An entry of `/proc/self/mountinfo`. +struct MountEntry { + device: DeviceId, + source: String, + mount_point: PathBuf, +} + +/// Parses a line of `/proc/self/mountinfo`: +/// +/// ```text +/// 26 25 8:2 / /boot rw,relatime shared:5 - ext4 /dev/sda2 rw +/// ``` +/// +/// The optional fields before the `-` separator vary in count, so the mount +/// source is located relative to it rather than by a fixed index. +fn parse_mount_entry(line: &str) -> Option { + let fields = line.split_whitespace().collect::>(); + let separator = fields.iter().position(|field| *field == "-")?; + let (major, minor) = fields.get(2)?.split_once(':')?; + + Some(MountEntry { + device: (major.parse().ok()?, minor.parse().ok()?), + source: fields.get(separator + 2)?.to_string(), + mount_point: PathBuf::from(fields.get(4)?), + }) +} + +/// Collects `device` along with every block device sharing its storage: the +/// partitions it contains, the disk it is a partition of and whatever is +/// stacked on top of it, such as LVM, MD and loop devices. +/// +/// Sibling partitions are deliberately left out, writing to one partition does +/// not reach the others. +fn related_block_devices(device: DeviceId) -> HashSet { + let mut found = HashSet::new(); + let mut pending = vec![device]; + + while let Some(device) = pending.pop() { + if !found.insert(device) { + continue; + } + + let sysfs = PathBuf::from(format!("/sys/dev/block/{}:{}", device.0, device.1)); + let Ok(sysfs) = sysfs.canonicalize() else { + continue; + }; + + // A partition's directory lives inside the one of its disk. The disk is + // recorded but not walked, otherwise its other partitions would be + // dragged in as well. + if sysfs.join("partition").exists() { + found.extend(sysfs.parent().and_then(|disk| read_device_id(&disk.join("dev")))); + } else { + pending.extend(device_ids_in(&sysfs, |path| path.join("partition").exists())); + } + + pending.extend(device_ids_in(&sysfs.join("holders"), |_| true)); + } + + found +} + +/// 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 { + return Vec::default(); + }; + + entries + .filter_map(std::result::Result::ok) + .map(|entry| entry.path()) + .filter(|path| wanted(path)) + .filter_map(|path| read_device_id(&path.join("dev"))) + .collect() +} + +fn read_device_id(path: &Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; + let (major, minor) = content.trim().split_once(':')?; + + Some((major.parse().ok()?, minor.parse().ok()?)) +} + pub(crate) fn is_executable_in_path(cmd: &str) -> Result<()> { trace!("checking if {} is executable", cmd); match quale::which(cmd) { @@ -46,6 +189,10 @@ pub(crate) fn is_executable_in_path(cmd: &str) -> Result<()> { } pub(crate) fn format(target: &Path, fs: Filesystem, options: &Option) -> Result<()> { + // The commands below are forced so they run unattended, which also means + // they will not refuse to wipe a mounted filesystem on their own. + ensure_not_mounted(target)?; + trace!("formating {:?} as {}", target, fs); let target = target.display(); let options = options.clone().unwrap_or_default(); @@ -102,3 +249,89 @@ pub(crate) fn chown(path: &Path, uid: &Option, gid: &Option) -> Result gid.as_ref().map(|id| nix::unistd::Gid::from_raw(id.as_u32())), )?) } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn mount_entry_without_optional_fields() { + let entry = parse_mount_entry("26 25 8:2 / /boot rw,relatime - ext4 /dev/sda2 rw").unwrap(); + + assert_eq!(entry.device, (8, 2)); + assert_eq!(entry.source, "/dev/sda2"); + assert_eq!(entry.mount_point, PathBuf::from("/boot")); + } + + #[test] + fn mount_entry_with_optional_fields() { + let entry = parse_mount_entry( + "26 25 0:23 / / rw,relatime shared:5 master:1 - ubifs ubi0:rootfs rw", + ) + .unwrap(); + + assert_eq!(entry.device, (0, 23)); + assert_eq!(entry.source, "ubi0:rootfs"); + assert_eq!(entry.mount_point, PathBuf::from("/")); + } + + #[test] + fn malformed_mount_entries_are_dropped() { + assert!(parse_mount_entry("").is_none()); + assert!(parse_mount_entry("26 25 8:2 / /boot rw,relatime ext4 /dev/sda2 rw").is_none()); + assert!(parse_mount_entry("26 25 bogus / /boot rw - ext4 /dev/sda2 rw").is_none()); + } + + #[test] + fn regular_file_is_not_in_use() { + let file = tempfile::NamedTempFile::new().unwrap(); + + assert!(ensure_not_mounted(file.path()).is_ok()); + } + + #[test] + fn missing_device_is_not_in_use() { + assert!(ensure_not_mounted(Path::new("/dev/updatehub-inexistent-device")).is_ok()); + } + + #[test] + #[ignore] + fn mounted_device_is_in_use() { + use std::io::{Seek, SeekFrom, Write}; + + // Generate a sparse file for the faked device use + let mut image = tempfile::NamedTempFile::new().unwrap(); + image.seek(SeekFrom::Start(32 * 1024 * 1024)).unwrap(); + image.write_all(&[0]).unwrap(); + + // Setup faked device + let (loopdev, device) = { + // Loop device next_free is not thread safe + let mutex = crate::object::installer::tests::SERIALIZE.clone(); + let _mutex = mutex.lock().unwrap(); + let loopdev = loopdev::LoopControl::open().unwrap().next_free().unwrap(); + let device = loopdev.path().unwrap(); + loopdev.attach_file(image.path()).unwrap(); + (loopdev, device) + }; + + assert!( + ensure_not_mounted(&device).is_ok(), + "an unmounted loop device should be free to install onto" + ); + + format(&device, Filesystem::Ext4, &None).unwrap(); + let guard = mount(&device, Filesystem::Ext4, "").unwrap(); + + let in_use = ensure_not_mounted(&device); + // Unmount before asserting so the loop device is always released. + drop(guard); + loopdev.detach().unwrap(); + + assert!( + matches!(in_use, Err(Error::DeviceInUse { .. })), + "a mounted device must be rejected, got {in_use:?}" + ); + } +} diff --git a/updatehub/src/utils/mod.rs b/updatehub/src/utils/mod.rs index 664f0bf4..9baea15d 100644 --- a/updatehub/src/utils/mod.rs +++ b/updatehub/src/utils/mod.rs @@ -33,6 +33,15 @@ pub enum Error { #[from(ignore)] MissingWritePermission(#[error(not(source))] std::path::PathBuf), + #[display( + "target device {device:?} is in use, it holds the filesystem mounted at {mount_point:?}" + )] + #[from(ignore)] + DeviceInUse { + device: std::path::PathBuf, + mount_point: std::path::PathBuf, + }, + #[display( "{available} is not enough storage space for installation, at least {required} is required" )] diff --git a/updatehub/src/utils/mtd.rs b/updatehub/src/utils/mtd.rs index 324b9b4c..f507a09e 100644 --- a/updatehub/src/utils/mtd.rs +++ b/updatehub/src/utils/mtd.rs @@ -7,7 +7,7 @@ use super::{Error, Result}; use std::{ fs, io::{BufRead, BufReader}, - path::PathBuf, + path::{Path, PathBuf}, }; pub(crate) fn target_device_from_ubi_volume_name(volume: &str) -> Result { @@ -58,6 +58,82 @@ pub(crate) fn target_device_from_mtd_name(name: &str) -> Result { .ok_or_else(|| Error::NoMtdDevice(name.to_owned())) } +/// Mount sources which would be taken down by writing to `device`, an MTD +/// character device or a UBI volume. +/// +/// Neither UBIFS nor JFFS2 mount a block device, so `/proc/self/mountinfo` +/// carries a name such as `ubi0:rootfs` or `mtd:rootfs` and the relationship +/// can only be reconstructed from it. +pub(crate) fn mount_sources_for(device: &Path) -> Vec { + let Some(name) = device.file_name().and_then(|name| name.to_str()) else { + return Vec::default(); + }; + + if name.starts_with("ubi") && name.contains('_') { + return ubi_volume_sources(name); + } + + match name.strip_prefix("mtd").map(str::parse) { + Some(Ok(number)) => mtd_sources(number), + _ => Vec::default(), + } +} + +/// Sources for a UBI volume, as in `ubi0_1`. +fn ubi_volume_sources(volume: &str) -> Vec { + let mut sources = vec![format!("/dev/{volume}"), volume.to_owned()]; + + // UBIFS is usually mounted through the volume name, as in `ubi0:rootfs`. + if let Some((device, _)) = volume.split_once('_') { + if let Ok(name) = fs::read_to_string(format!("/sys/class/ubi/{volume}/name")) { + sources.push(format!("{device}:{}", name.trim())); + } + } + + sources +} + +fn mtd_sources(number: u32) -> Vec { + // mountinfo records the source as it was passed to mount(2), so the bare + // names are kept alongside the absolute ones to cover a relative mount. + let mut sources = vec![ + format!("/dev/mtd{number}"), + format!("mtd{number}"), + format!("/dev/mtdblock{number}"), + format!("mtdblock{number}"), + ]; + + if let Ok(name) = fs::read_to_string(format!("/sys/class/mtd/mtd{number}/name")) { + sources.push(format!("mtd:{}", name.trim())); + } + + // A UBI device attached to this MTD keeps its volumes on it, so erasing the + // MTD takes every mounted volume down as well. + sources.extend(ubi_volumes_on_mtd(number).iter().flat_map(|volume| ubi_volume_sources(volume))); + + sources +} + +/// Names of the UBI volumes, as in `ubi0_1`, stored on the given MTD device. +fn ubi_volumes_on_mtd(number: u32) -> Vec { + let Ok(entries) = fs::read_dir("/sys/class/ubi") else { + return Vec::default(); + }; + + // `/sys/class/ubi` lists both the UBI devices, `ubi0`, and their volumes, + // `ubi0_1`, so the device a volume sits on is read off its own name. + entries + .filter_map(std::result::Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|volume| { + volume.split_once('_').is_some_and(|(device, _)| { + fs::read_to_string(format!("/sys/class/ubi/{device}/mtd_num")) + .is_ok_and(|mtd| mtd.trim().parse() == Ok(number)) + }) + }) + .collect() +} + mod ffi { use crate::utils::Result; use nix::ioctl_read;