From 65ad0d630c0386d1d4d02ea4acee4c3edf57e843 Mon Sep 17 00:00:00 2001 From: Ariel Miculas-Trif Date: Thu, 3 Oct 2024 22:57:19 +0300 Subject: [PATCH 1/3] Enable puzzlefs to be used as a MOUNT_HELPER in the LXC OCI template The mount-helper is called like this: mount-helper mount --writable : mount-helper umount I've added the support for the `--writable` flag that creates a writable overlay on top of a PuzzleFS mountpoint (used as a lowerdir). This requires root privileges. I've also added the support for the `umount` subcommand which detects whether the puzzlefs filesystem was mounted as a read-only fuse driver or whether an overlay was created on top of it, and handles the unmounting accordingly. Unmounting an overlay requires root privileges. I've implemented the changes on the LXC side in [1]. [1] https://github.com/lxc/lxc/pull/4483 Signed-off-by: Ariel Miculas-Trif --- Cargo.lock | 81 +++++++++++++++++++++++++------- exe/Cargo.toml | 3 +- exe/src/main.rs | 122 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aa21a8f..a65273b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -135,6 +135,12 @@ dependencies = [ "serde", ] +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "bitflags" version = "2.6.0" @@ -266,6 +272,12 @@ dependencies = [ "shlex", ] +[[package]] +name = "cfg-if" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" + [[package]] name = "cfg-if" version = "1.0.0" @@ -359,7 +371,7 @@ version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a97769d94ddab943e4510d138150169a2758b5ef3eb191a9ee688de3e23ef7b3" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", ] [[package]] @@ -560,7 +572,7 @@ version = "0.2.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35c0522e981e68cbfa8c3f978441a5f34b30b96e146b33cd3359176b50fe8586" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "libredox", "windows-sys 0.59.0", @@ -652,7 +664,7 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4567c8db10ae91089c99af84c68c38da3ec2f087c3f82960bcdbf3656b6f4d7" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "libc", "wasi", ] @@ -816,13 +828,24 @@ version = "0.2.158" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d8adc4bb1803a324070e64a98ae98f38934d91957a99cfb3a43dcbc01bc56439" +[[package]] +name = "libmount" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23c4c2ad2d5cbd2f5a05620c3daf45930add53ec207fa99ce5eec971089dc35f" +dependencies = [ + "libc", + "nix 0.14.1", + "quick-error", +] + [[package]] name = "libredox" version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ - "bitflags", + "bitflags 2.6.0", "libc", "redox_syscall", ] @@ -886,14 +909,27 @@ dependencies = [ "adler2", ] +[[package]] +name = "nix" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c722bee1037d430d0f8e687bbdbf222f27cc6e4e68d5caf630857bb2b6dbdce" +dependencies = [ + "bitflags 1.3.2", + "cc", + "cfg-if 0.1.10", + "libc", + "void", +] + [[package]] name = "nix" version = "0.27.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2eb04e9c688eff1c89d72b407f168cf79bb9e867a9d3323ed6c01519eb9cc053" dependencies = [ - "bitflags", - "cfg-if", + "bitflags 2.6.0", + "cfg-if 1.0.0", "libc", ] @@ -903,8 +939,8 @@ version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" dependencies = [ - "bitflags", - "cfg-if", + "bitflags 2.6.0", + "cfg-if 1.0.0", "cfg_aliases", "libc", ] @@ -1032,8 +1068,8 @@ version = "0.10.66" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9529f4786b70a3e8c61e11179af17ab6188ad8d0ded78c5529441ed39d4bd9c1" dependencies = [ - "bitflags", - "cfg-if", + "bitflags 2.6.0", + "cfg-if 1.0.0", "foreign-types", "libc", "once_cell", @@ -1211,6 +1247,7 @@ dependencies = [ "dir-diff", "env_logger", "hex", + "libmount", "log", "nix 0.27.1", "os_pipe", @@ -1249,6 +1286,12 @@ dependencies = [ "zstd-seekable", ] +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quote" version = "1.0.37" @@ -1294,7 +1337,7 @@ version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0884ad60e090bf1345b93da0a5de8923c93884cd03f40dfcfddd3b4bee661853" dependencies = [ - "bitflags", + "bitflags 2.6.0", ] [[package]] @@ -1338,7 +1381,7 @@ version = "0.38.36" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f55e80d50763938498dd5ebb18647174e0c76dc38c5505294bb224624f30f36" dependencies = [ - "bitflags", + "bitflags 2.6.0", "errno", "itoa", "libc", @@ -1406,7 +1449,7 @@ version = "0.10.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "cpufeatures", "digest", ] @@ -1512,7 +1555,7 @@ version = "3.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04cbcdd0c794ebb0d4cf35e88edd2f7d2c4c3e9a5a6dab322839b321c6a87a64" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "fastrand", "once_cell", "rustix", @@ -1676,6 +1719,12 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "void" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" + [[package]] name = "wait-timeout" version = "0.2.0" @@ -1707,7 +1756,7 @@ version = "0.2.93" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a82edfc16a6c469f5f44dc7b571814045d60404b55a0ee849f9bcfa2e63dd9b5" dependencies = [ - "cfg-if", + "cfg-if 1.0.0", "once_cell", "wasm-bindgen-macro", ] @@ -1893,7 +1942,7 @@ version = "0.36.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f9643b83820c0cd246ecabe5fa454dd04ba4fa67996369466d0747472d337346" dependencies = [ - "bitflags", + "bitflags 2.6.0", "windows-sys 0.52.0", ] diff --git a/exe/Cargo.toml b/exe/Cargo.toml index d3b5332..1da147b 100644 --- a/exe/Cargo.toml +++ b/exe/Cargo.toml @@ -15,7 +15,7 @@ edition = "2021" [dependencies] anyhow = "1.0.75" -nix = "0.27.1" +nix = {version = "0.27.1", features = ["mount"] } clap = { version = "4.0.18", features = ["derive"] } # Version 0.5 drops exit_action so we're stuck with 0.4 daemonize = "0.4.1" @@ -26,6 +26,7 @@ syslog = "6.0.1" os_pipe = "1.1.2" puzzlefs-lib = { path = "../puzzlefs-lib", version = "0.2.0" } hex = "0.4.3" +libmount = "0.1.15" [dev-dependencies] assert_cmd = "2.0.12" diff --git a/exe/src/main.rs b/exe/src/main.rs index b77e2e6..37aa24c 100644 --- a/exe/src/main.rs +++ b/exe/src/main.rs @@ -1,7 +1,11 @@ use clap::{Args, Parser, Subcommand}; use daemonize::Daemonize; use env_logger::Env; +use libmount::mountinfo; +use libmount::Overlay; use log::{error, info, LevelFilter}; +use nix::mount::umount; +use nix::unistd::Uid; use os_pipe::{PipeReader, PipeWriter}; use puzzlefs_lib::{ builder::{add_rootfs_delta, build_initial_rootfs, enable_fs_verity}, @@ -11,6 +15,7 @@ use puzzlefs_lib::{ oci::Image, reader::{fuse::PipeDescriptor, mount, spawn_mount}, }; +use std::ffi::{OsStr, OsString}; use std::fs; use std::fs::OpenOptions; use std::io::prelude::*; @@ -30,6 +35,7 @@ struct Opts { enum SubCommand { Build(Build), Mount(Mount), + Umount(Umount), Extract(Extract), EnableFsVerity(FsVerity), } @@ -56,6 +62,13 @@ struct Mount { options: Option>, #[arg(short, long, value_name = "fs verity root digest")] digest: Option, + #[arg(short, long, conflicts_with = "foreground")] + writable: bool, +} + +#[derive(Args)] +struct Umount { + mountpoint: String, } #[derive(Args)] @@ -106,6 +119,7 @@ fn init_syslog(log_level: &str) -> std::io::Result<()> { Ok(()) } +#[allow(clippy::too_many_arguments)] fn mount_background( image: Image, tag: &str, @@ -114,6 +128,7 @@ fn mount_background( manifest_verity: Option>, mut recv: PipeReader, init_notify: &PipeWriter, + parent_action: impl FnOnce() -> anyhow::Result<()> + 'static, ) -> anyhow::Result<()> { let daemonize = Daemonize::new().exit_action(move || { let mut read_buffer = [0]; @@ -124,6 +139,9 @@ fn mount_background( // we explicitly exit with an error code, otherwise exit(0) is done by daemonize exit(1); } + if let Err(e) = parent_action() { + error!("parent_action error {e}"); + } }); match daemonize.start() { @@ -153,6 +171,20 @@ fn parse_oci_dir(oci_dir: &str) -> anyhow::Result<(&str, &str)> { Ok((components[0], components[1])) } +fn get_mount_type(mountpoint: &str) -> anyhow::Result { + let contents = fs::read_to_string("/proc/self/mountinfo")?; + let mut parser = mountinfo::Parser::new(contents.as_bytes()); + let mount_info = parser.find(|mount_info| { + mount_info + .as_ref() + .map(|mount_info| mount_info.mount_point == OsStr::new(mountpoint)) + .unwrap_or(false) + }); + let mount_info = mount_info + .ok_or_else(|| anyhow::anyhow!("cannot find mountpoint in /proc/self/mountpoints"))??; + Ok(mount_info.fstype.into_owned()) +} + fn main() -> anyhow::Result<()> { let opts: Opts = Opts::parse(); match opts.subcmd { @@ -197,6 +229,10 @@ fn main() -> anyhow::Result<()> { init_syslog(log_level)?; } + if m.writable && !Uid::effective().is_root() { + anyhow::bail!("Writable mounts can only be created by the root user!") + } + let (oci_dir, tag) = parse_oci_dir(&m.oci_dir)?; let oci_dir = Path::new(oci_dir); let oci_dir = fs::canonicalize(oci_dir)?; @@ -206,6 +242,43 @@ fn main() -> anyhow::Result<()> { let manifest_verity = m.digest.map(hex::decode).transpose()?; + if m.writable { + // We only support background mounts with the writable flag + let (recv, mut init_notify) = os_pipe::pipe()?; + let pfs_mountpoint = mountpoint.join("ro"); + fs::create_dir_all(&pfs_mountpoint)?; + + if let Err(e) = mount_background( + image, + tag, + &pfs_mountpoint.clone(), + m.options, + manifest_verity, + recv, + &init_notify, + move || { + let ovl_workdir = mountpoint.join("work"); + fs::create_dir_all(&ovl_workdir)?; + let ovl_upperdir = mountpoint.join("upper"); + fs::create_dir_all(&ovl_upperdir)?; + let overlay = Overlay::writable( + [pfs_mountpoint.as_path()].into_iter(), + ovl_upperdir, + ovl_workdir, + &mountpoint, + ); + overlay.mount().map_err(|e| anyhow::anyhow!("{e}")) + }, + ) { + if let Err(e) = init_notify.write_all(b"f") { + error!("puzzlefs will hang because we couldn't write to pipe, {e}"); + } + error!("mount_background failed: {e}"); + return Err(e); + } + return Ok(()); + } + if m.foreground { let (send, recv) = std::sync::mpsc::channel(); let send_ctrlc = send.clone(); @@ -257,6 +330,7 @@ fn main() -> anyhow::Result<()> { manifest_verity, recv, &init_notify, + || Ok(()), ) { if let Err(e) = init_notify.write_all(b"f") { error!("puzzlefs will hang because we couldn't write to pipe, {e}"); @@ -268,6 +342,54 @@ fn main() -> anyhow::Result<()> { Ok(()) } + SubCommand::Umount(e) => { + let mountpoint = Path::new(&e.mountpoint); + let mount_type = get_mount_type(&e.mountpoint)?; + match mount_type.to_str() { + Some("overlay") => { + if !Uid::effective().is_root() { + anyhow::bail!("Overlay mounts can only be unmounted by the root user!") + } + umount(mountpoint)?; + // Now unmount the read-only puzzlefs mountpoint + let pfs_mountpoint = mountpoint.join("ro"); + umount(pfs_mountpoint.as_os_str())?; + // TODO: Decide whether to remove the directories we've created. For the LXC + // case, we don't want to remove them because we want to persist state between + // multiple mounts. Should we add a --delete flag to unmount? + // let ovl_workdir = mountpoint.join("work"); + // let ovl_upperdir = mountpoint.join("upper"); + // std::fs::remove_dir_all(&pfs_mountpoint)?; + // std::fs::remove_dir_all(&ovl_workdir)?; + // std::fs::remove_dir_all(&ovl_upperdir)?; + return Ok(()); + } + Some("fuse") => { + // We call "fusermount -u" because we don't have permissions to umount directly + // fusermount and umount binaries have the setuid bit set + let status = std::process::Command::new("fusermount") + .arg("-u") + .arg(&e.mountpoint) + .status()?; + if !status.success() { + anyhow::bail!( + "umount exited with status {}", + status + .code() + .map(|code| code.to_string()) + .unwrap_or("terminated by signal".to_string()) + ); + } + } + _ => anyhow::bail!( + "Unknown mountpoint type {} for {}", + mount_type.to_str().unwrap_or("unknown mount type"), + &e.mountpoint + ), + } + + Ok(()) + } SubCommand::Extract(e) => { let (oci_dir, tag) = parse_oci_dir(&e.oci_dir)?; init_logging("info"); From a62c2b044f28b2c22b1ce2f1f318768d997fb1b7 Mon Sep 17 00:00:00 2001 From: Ariel Miculas-Trif Date: Fri, 4 Oct 2024 21:47:57 +0300 Subject: [PATCH 2/3] Add --persist flag to allow the user to specify the overlay's upperdir Signed-off-by: Ariel Miculas-Trif --- exe/src/main.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/exe/src/main.rs b/exe/src/main.rs index 37aa24c..a63a09f 100644 --- a/exe/src/main.rs +++ b/exe/src/main.rs @@ -64,6 +64,8 @@ struct Mount { digest: Option, #[arg(short, long, conflicts_with = "foreground")] writable: bool, + #[arg(short, long, conflicts_with = "foreground")] + persist: Option, } #[derive(Args)] @@ -229,7 +231,7 @@ fn main() -> anyhow::Result<()> { init_syslog(log_level)?; } - if m.writable && !Uid::effective().is_root() { + if (m.writable || m.persist.is_some()) && !Uid::effective().is_root() { anyhow::bail!("Writable mounts can only be created by the root user!") } @@ -242,8 +244,8 @@ fn main() -> anyhow::Result<()> { let manifest_verity = m.digest.map(hex::decode).transpose()?; - if m.writable { - // We only support background mounts with the writable flag + if m.writable || m.persist.is_some() { + // We only support background mounts with the writable|persist flag let (recv, mut init_notify) = os_pipe::pipe()?; let pfs_mountpoint = mountpoint.join("ro"); fs::create_dir_all(&pfs_mountpoint)?; @@ -259,7 +261,10 @@ fn main() -> anyhow::Result<()> { move || { let ovl_workdir = mountpoint.join("work"); fs::create_dir_all(&ovl_workdir)?; - let ovl_upperdir = mountpoint.join("upper"); + let ovl_upperdir = match m.persist { + None => mountpoint.join("upper"), + Some(upperdir) => Path::new(&upperdir).to_path_buf(), + }; fs::create_dir_all(&ovl_upperdir)?; let overlay = Overlay::writable( [pfs_mountpoint.as_path()].into_iter(), From 1db9958bb18ccab9727dad227cf62d0c6a5b5a7c Mon Sep 17 00:00:00 2001 From: Ariel Miculas-Trif Date: Thu, 3 Oct 2024 23:17:22 +0300 Subject: [PATCH 3/3] Latest clippy has spoken! Fix the following errors reported by clippy: ``` error: manual arithmetic check found --> puzzlefs-lib/src/reader/puzzlefs.rs:45:27 | 45 | let addl_offset = if offset > file_offset { | ___________________________^ 46 | | offset - file_offset 47 | | } else { 48 | | 0 49 | | }; | |_________^ help: replace it with: `offset.saturating_sub(file_offset)` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub = note: `-D clippy::implicit-saturating-sub` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::implicit_saturating_sub)]` ``` and ignore the needless-lifetimes errors reported on the autogenerated capnp code: ``` | 15 | impl <'a,> ::core::marker::Copy for Reader<'a,> {} | ^^ ^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes = note: `-D clippy::needless-lifetimes` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::needless_lifetimes)]` help: elide the lifetimes | 15 - impl <'a,> ::core::marker::Copy for Reader<'a,> {} 15 + impl ::core::marker::Copy for Reader<'_,> {} | ``` Signed-off-by: Ariel Miculas-Trif --- puzzlefs-lib/src/compression/zstd_seekable_wrapper.rs | 6 +++--- puzzlefs-lib/src/format/types.rs | 2 +- puzzlefs-lib/src/lib.rs | 1 + puzzlefs-lib/src/reader/puzzlefs.rs | 6 +----- puzzlefs-lib/src/reader/walk.rs | 2 +- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/puzzlefs-lib/src/compression/zstd_seekable_wrapper.rs b/puzzlefs-lib/src/compression/zstd_seekable_wrapper.rs index d2cad25..36ea047 100644 --- a/puzzlefs-lib/src/compression/zstd_seekable_wrapper.rs +++ b/puzzlefs-lib/src/compression/zstd_seekable_wrapper.rs @@ -67,13 +67,13 @@ pub struct ZstdDecompressor<'a, R: Read + Seek> { uncompressed_length: u64, } -impl<'a, R: Seek + Read> Decompressor for ZstdDecompressor<'a, R> { +impl Decompressor for ZstdDecompressor<'_, R> { fn get_uncompressed_length(&mut self) -> io::Result { Ok(self.uncompressed_length) } } -impl<'a, R: Seek + Read> Seek for ZstdDecompressor<'a, R> { +impl Seek for ZstdDecompressor<'_, R> { fn seek(&mut self, offset: io::SeekFrom) -> io::Result { match offset { io::SeekFrom::Start(s) => { @@ -97,7 +97,7 @@ impl<'a, R: Seek + Read> Seek for ZstdDecompressor<'a, R> { } } -impl<'a, R: Seek + Read> Read for ZstdDecompressor<'a, R> { +impl Read for ZstdDecompressor<'_, R> { fn read(&mut self, out: &mut [u8]) -> io::Result { // decompress() gets angry (ZSTD("Corrupted block detected")) if you pass it a buffer // longer than the uncompressable data, so let's be careful to truncate the buffer if it diff --git a/puzzlefs-lib/src/format/types.rs b/puzzlefs-lib/src/format/types.rs index 385c3d8..25fc771 100644 --- a/puzzlefs-lib/src/format/types.rs +++ b/puzzlefs-lib/src/format/types.rs @@ -896,7 +896,7 @@ impl<'de> Deserialize<'de> for Digest { { struct DigestVisitor; - impl<'de> Visitor<'de> for DigestVisitor { + impl Visitor<'_> for DigestVisitor { type Value = Digest; fn expecting(&self, formatter: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { diff --git a/puzzlefs-lib/src/lib.rs b/puzzlefs-lib/src/lib.rs index 80f882b..4c96f88 100644 --- a/puzzlefs-lib/src/lib.rs +++ b/puzzlefs-lib/src/lib.rs @@ -12,6 +12,7 @@ pub mod fsverity_helpers; pub mod oci; pub mod reader; +#[allow(clippy::needless_lifetimes)] pub mod metadata_capnp { include!(concat!(env!("OUT_DIR"), "/metadata_capnp.rs")); } diff --git a/puzzlefs-lib/src/reader/puzzlefs.rs b/puzzlefs-lib/src/reader/puzzlefs.rs index 68b0fa6..048e8c5 100644 --- a/puzzlefs-lib/src/reader/puzzlefs.rs +++ b/puzzlefs-lib/src/reader/puzzlefs.rs @@ -42,11 +42,7 @@ pub(crate) fn file_read( continue; } - let addl_offset = if offset > file_offset { - offset - file_offset - } else { - 0 - }; + let addl_offset = offset.saturating_sub(file_offset); // ok, need to read this chunk; how much? let left_in_buf = data.len() - buf_offset; diff --git a/puzzlefs-lib/src/reader/walk.rs b/puzzlefs-lib/src/reader/walk.rs index 008706f..8ff1376 100644 --- a/puzzlefs-lib/src/reader/walk.rs +++ b/puzzlefs-lib/src/reader/walk.rs @@ -48,7 +48,7 @@ impl<'a> WalkPuzzleFS<'a> { } } -impl<'a> Iterator for WalkPuzzleFS<'a> { +impl Iterator for WalkPuzzleFS<'_> { type Item = Result; fn next(&mut self) -> Option {