From 299c2cff8a697843beecb5fdc5cd5d050bc6bd7b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 17 May 2024 16:04:17 -0400 Subject: [PATCH 1/2] rootmap: correctly join paths when looking for physical root To iterate faster, it's convenient to be able to run `rdcore rootmap` directly from the real root (mounted over from the development host) and pointing it at `/`. But the naive appending we do here to find the physical root would give `//sysroot` which isn't in the mount table (obviously we could also canonicalize, but it's just cleaner to join it correctly in the first place). --- src/bin/rdcore/rootmap.rs | 12 +++++++----- src/blockdev.rs | 8 ++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/bin/rdcore/rootmap.rs b/src/bin/rdcore/rootmap.rs index 765c99f41..eee7108e9 100644 --- a/src/bin/rdcore/rootmap.rs +++ b/src/bin/rdcore/rootmap.rs @@ -31,11 +31,12 @@ use crate::cmdline::*; const PHYSICAL_ROOT_MOUNT: &str = "sysroot"; pub fn rootmap(config: RootmapConfig) -> Result<()> { + let root_mount_path = Path::new(&config.root_mount); // Get the mount point for the deployment root, which will have e.g. /etc which we might parse - let rootfs_mount = Mount::from_existing(&config.root_mount)?; + let rootfs_mount = Mount::from_existing(root_mount_path)?; // get the backing device for the "physical" root - let physical_root_path = format!("{}/{PHYSICAL_ROOT_MOUNT}", config.root_mount); - let physical_mount = Mount::from_existing(&physical_root_path)?; + let physical_root_path = root_mount_path.join(PHYSICAL_ROOT_MOUNT); + let physical_mount = Mount::from_existing(physical_root_path)?; let device = PathBuf::from(physical_mount.device()); // and from that we can collect all the parent backing devices too @@ -233,10 +234,11 @@ fn get_luks_uuid(device: &Path) -> Result { } pub fn bind_boot(config: BindBootConfig) -> Result<()> { + let root_mount_path = Path::new(&config.root_mount); let boot_mount = Mount::from_existing(&config.boot_mount)?; // We always operate here on the physical root - let physical_root_path = format!("{}/{PHYSICAL_ROOT_MOUNT}", config.root_mount); - let root_mount = Mount::from_existing(&physical_root_path)?; + let physical_root_path = root_mount_path.join(PHYSICAL_ROOT_MOUNT); + let root_mount = Mount::from_existing(physical_root_path)?; let boot_uuid = boot_mount.get_filesystem_uuid()?; let root_uuid = root_mount.get_filesystem_uuid()?; diff --git a/src/blockdev.rs b/src/blockdev.rs index 9aa71297e..cb7fe5510 100644 --- a/src/blockdev.rs +++ b/src/blockdev.rs @@ -447,7 +447,7 @@ impl Mount { }) } - pub fn from_existing(path: &str) -> Result { + pub fn from_existing>(path: P) -> Result { let mounts = read_to_string("/proc/self/mounts").context("reading mount table")?; for line in mounts.lines() { let mount: Vec<&str> = line.split_whitespace().collect(); @@ -455,15 +455,15 @@ impl Mount { if mount.len() != 6 { bail!("invalid line in /proc/self/mounts: {}", line); } - if mount[1] == path { + if Path::new(mount[1]) == path.as_ref() { return Ok(Mount { device: mount[0].to_string(), - mountpoint: path.into(), + mountpoint: path.as_ref().into(), owned: false, }); } } - bail!("mountpoint {} not found", path); + bail!("mountpoint {} not found", path.as_ref().display()); } pub fn device(&self) -> &str { From 0d42e101814674d480d6df589d3289bc835b033b Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 17 May 2024 18:23:51 -0400 Subject: [PATCH 2/2] rootmap: fix multipath + LUKS case In the multipath + LUKS case, `get_luks_uuid()` would incorrectly skip over the multipath partition containing the LUKS header because `is_dm_device()` returned true. The code eventually errors out when it gets to the disks backing the multipath device. The `is_dm_device()` check was added as part of 69b706d ("rootmap: handle filesystems with LUKS integrity") to correctly handle the LUKS integrity case in the Secure Execution path. There, the device right under the LUKS device is another crypt device mapper device used for integrity that we need to skip over. Instead of generically checking for a device mapper target, check specifically that it's a LUKS integrity target before deciding to skip. Part of: https://github.com/coreos/fedora-coreos-tracker/issues/1728 Co-authored-by: Aashish Radhakrishnan Co-authored-by: Gursewak Mangat Co-authored-by: Michael Nguyen Co-authored-by: Steven Presti --- docs/release-notes.md | 1 + src/bin/rdcore/rootmap.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 7c57c869f..bc21f13dd 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,6 +14,7 @@ Minor changes: Internal changes: +- rootmap: Fix support for LUKS on multipath Packaging changes: diff --git a/src/bin/rdcore/rootmap.rs b/src/bin/rdcore/rootmap.rs index eee7108e9..43b466c35 100644 --- a/src/bin/rdcore/rootmap.rs +++ b/src/bin/rdcore/rootmap.rs @@ -219,7 +219,7 @@ fn get_luks_uuid(device: &Path) -> Result { match deps.as_slice() { [] => bail!("missing parent device for {}", device.display()), [device] => { - if Disk::new(device)?.is_dm_device() { + if Disk::new(device)?.is_luks_integrity()? { return get_luks_uuid(device); } Ok(runcmd_output!("cryptsetup", "luksUUID", device)?