Skip to content

Commit

Permalink
efi: update the ESP by creating a tmpdir and RENAME_EXCHANGE
Browse files Browse the repository at this point in the history 
See Timothée's comment coreos#454 (comment)
Reuse `TMP_PREFIX`, logic is like this:
- `cp -a fedora .btmp.fedora`
  - We start with a copy to make sure to keep all other files
that we do not explicitly track in bootupd
- Update the content of `.btmp.fedora` with the new binaries
- Exchange `.btmp.fedora` -> `fedora`
- Remove now "old" `.btmp.fedora`

If we have a file not in a directory in `EFI`, then we can copy
it to `.btmp.foo` and then act on it and finally rename it. No
need to copy the entire `EFI`.

And use `insert()` instead of `push()` to match `starts_with()`
when scanning temp files & dirs.
Fixes coreos#454
  • Loading branch information
@HuijingHei
HuijingHei committed Jul 9, 2024
1 parent 8a472ea commit 7d124de
Show file tree
Hide file tree
Showing 3 changed files with 248 additions and 34 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ path = "src/main.rs"
[dependencies]
anyhow = "1.0"
bincode = "1.3.2"
camino = "1.1.7"
chrono = { version = "0.4.38", features = ["serde"] }
clap = { version = "3.2", default-features = false, features = ["cargo", "derive", "std", "suggestions"] }
env_logger = "0.10"
Expand Down
274 changes: 240 additions & 34 deletions src/filetree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use anyhow::{bail, Context, Result};
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use camino::{Utf8Path, Utf8PathBuf};
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use openat_ext::OpenatDirExt;
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use openssl::hash::{Hasher, MessageDigest};
use rustix::fd::BorrowedFd;
use serde::{Deserialize, Serialize};
#[allow(unused_imports)]
use std::collections::{BTreeMap, HashMap, HashSet};
use std::fmt::Display;
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use std::os::unix::io::AsRawFd;
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
use std::path::Path;
use std::os::unix::process::CommandExt;
use std::process::Command;

/// The prefix we apply to our temporary files.
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
Expand Down Expand Up @@ -231,7 +234,7 @@ impl FileTree {
}
}

// Recursively remove all files in the directory that start with our TMP_PREFIX
// Recursively remove all files/dirs in the directory that start with our TMP_PREFIX
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
fn cleanup_tmp(dir: &openat::Dir) -> Result<()> {
for entry in dir.list_dir(".")? {
Expand All @@ -245,8 +248,13 @@ fn cleanup_tmp(dir: &openat::Dir) -> Result<()> {

match dir.get_file_type(&entry)? {
openat::SimpleType::Dir => {
let child = dir.sub_dir(name)?;
cleanup_tmp(&child)?;
if name.starts_with(TMP_PREFIX) {
dir.remove_all(name)?;
continue;
} else {
let child = dir.sub_dir(name)?;
cleanup_tmp(&child)?;
}
}
openat::SimpleType::File => {
if name.starts_with(TMP_PREFIX) {
Expand All @@ -272,20 +280,41 @@ pub(crate) struct ApplyUpdateOptions {
// Let's just fork off a helper process for now.
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> {
use rustix::fd::BorrowedFd;
use rustix::fs::{Mode, OFlags};
let d = unsafe { BorrowedFd::borrow_raw(d.as_raw_fd()) };
let oflags = OFlags::RDONLY | OFlags::CLOEXEC | OFlags::DIRECTORY;
let d = rustix::fs::openat(d, ".", oflags, Mode::empty())?;
rustix::fs::syncfs(d).map_err(Into::into)
}

/// Copy from src to dst at root dir
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
fn tmpname_for_path<P: AsRef<Path>>(path: P) -> std::path::PathBuf {
let path = path.as_ref();
let mut buf = path.file_name().expect("filename").to_os_string();
buf.push(TMP_PREFIX);
path.with_file_name(buf)
fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
let r = unsafe {
Command::new("cp")
.args(["-a"])
.arg(src)
.arg(dst)
.pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
.status()?
};
if !r.success() {
anyhow::bail!("Failed to copy {src} to {dst}");
}
log::debug!("Copy {src} to {dst}");
Ok(())
}

/// Get first sub dir and tmp sub dir for the path
/// "fedora/foo/bar" -> ("fedora", ".btmp.fedora")
/// "foo" -> ("foo", ".btmp.foo")
#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
fn get_subdir(path: &Utf8Path) -> Result<(String, String)> {
let buf = path.iter().next().unwrap().to_owned();
let mut buf_tmp = buf.clone();
buf_tmp.insert_str(0, TMP_PREFIX);
Ok((buf, buf_tmp))
}

/// Given two directories, apply a diff generated from srcdir to destdir
Expand All @@ -302,41 +331,94 @@ pub(crate) fn apply_diff(
let opts = opts.unwrap_or(&default_opts);
cleanup_tmp(destdir).context("cleaning up temporary files")?;

// Write new and changed files
for pathstr in diff.additions.iter().chain(diff.changes.iter()) {
let path = Path::new(pathstr);
if let Some(parent) = path.parent() {
destdir.ensure_dir_all(parent, DEFAULT_FILE_MODE)?;
let mut updates = HashMap::new();
// Handle removals in temp dir, or remove file directly if not in dir
if !opts.skip_removals {
for pathstr in diff.removals.iter() {
let path = Utf8Path::new(pathstr);
let (subdir, subdir_tmp) = get_subdir(path)?;
let path_tmp;
if subdir != *pathstr {
// need to copy to temp subdir and remember
if !destdir.exists(&subdir_tmp)? {
copy_dir(destdir, &subdir, &subdir_tmp)?;
updates.insert(subdir.clone(), subdir_tmp.clone());
}
path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
} else {
path_tmp = path.to_path_buf();
}
destdir
.remove_file(path_tmp.as_std_path())
.with_context(|| format!("removing {:?}", path_tmp))?;
}
let destp = tmpname_for_path(path);
srcdir
.copy_file_at(path, destdir, destp.as_path())
.with_context(|| format!("writing {}", &pathstr))?;
}
// Ensure all of the new files are written persistently to disk
if !opts.skip_sync {
syncfs(destdir)?;
// Write changed files to temp dir or temp file
for pathstr in diff.changes.iter() {
let path = Utf8Path::new(pathstr);
let (subdir, subdir_tmp) = get_subdir(path)?;
let mut path_tmp = Utf8PathBuf::from(&subdir_tmp);
if subdir != *pathstr {
if !destdir.exists(&subdir_tmp)? {
// copy to temp subdir if not exists
copy_dir(destdir, &subdir, &subdir_tmp)?;
}
path_tmp = path_tmp.join(path.strip_prefix(&subdir)?);
destdir
.remove_file(path_tmp.as_std_path())
.with_context(|| format!("removing {path_tmp} before copying"))?;
}
updates.insert(subdir.clone(), subdir_tmp.clone());
srcdir
.copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
.with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
}
// Now move them all into place (TODO track interruption)
for path in diff.additions.iter().chain(diff.changes.iter()) {
let pathtmp = tmpname_for_path(path);
destdir
.local_rename(&pathtmp, path)
.with_context(|| format!("renaming {path}"))?;
// Write new files to temp dir or temp file
for pathstr in diff.additions.iter() {
let path = Utf8Path::new(pathstr);
let (subdir, subdir_tmp) = get_subdir(path)?;
let mut path_tmp = Utf8PathBuf::from(&subdir_tmp);
if subdir != *pathstr {
path_tmp = path_tmp.join(path.strip_prefix(&subdir)?);
// ensure new additions dir exists
if let Some(parent) = path_tmp.parent().filter(|&v| !v.as_os_str().is_empty()) {
destdir.ensure_dir_all(parent.as_std_path(), DEFAULT_FILE_MODE)?;
}
}
updates.insert(subdir.clone(), subdir_tmp.clone());
srcdir
.copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
.with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
}
if !opts.skip_removals {
for path in diff.removals.iter() {

// do local exchange / rename
for (dst, tmp) in updates.iter() {
log::trace!("doing local exchange/rename for {} and {}", tmp, dst);
if destdir.exists(dst)? {
destdir
.local_exchange(tmp, dst)
.with_context(|| format!("exchange for {} and {}", tmp, dst))?;
} else {
destdir
.remove_file_optional(path)
.with_context(|| format!("removing {path}"))?;
.local_rename(tmp, dst)
.with_context(|| format!("rename for {} and {}", tmp, dst))?;
}
}
// Ensure all of the updates & changes are written persistently to disk
if !opts.skip_sync {
syncfs(destdir)?;
}

// finally remove the temp dir
for (_, tmp) in updates.iter() {
log::trace!("cleanup: {}", tmp);
destdir.remove_all(tmp).context("clean up temp")?;
}
// A second full filesystem sync to narrow any races rather than
// waiting for writeback to kick in.
if !opts.skip_sync {
syncfs(destdir)?;
}

Ok(())
}

Expand All @@ -345,6 +427,7 @@ mod tests {
use super::*;
use std::fs;
use std::io::Write;
use std::path::Path;

fn run_diff(a: &openat::Dir, b: &openat::Dir) -> Result<FileTreeDiff> {
let ta = FileTree::new_from_dir(a)?;
Expand Down Expand Up @@ -508,4 +591,127 @@ mod tests {
assert!(!a.join(relp).join("shim.x64").exists());
Ok(())
}
#[test]
fn test_get_subdir() -> Result<()> {
// test path
let path = Utf8Path::new("foo/subdir/bar");
let (tp, tp_tmp) = get_subdir(path)?;
assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
// test file
let path = Utf8Path::new("foo");
let (tp, tp_tmp) = get_subdir(path)?;
assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
Ok(())
}
#[test]
fn test_cleanup_tmp() -> Result<()> {
let tmpd = tempfile::tempdir()?;
let p = tmpd.path();
let pa = p.join("a/.btmp.a");
let pb = p.join(".btmp.b/b");
std::fs::create_dir_all(&pa)?;
std::fs::create_dir_all(&pb)?;
let dp = openat::Dir::open(p)?;
{
let mut buf = dp.write_file("a/foo", 0o644)?;
buf.write_all("foocontents".as_bytes())?;
let mut buf = dp.write_file("a/.btmp.foo", 0o644)?;
buf.write_all("foocontents".as_bytes())?;
let mut buf = dp.write_file(".btmp.b/foo", 0o644)?;
buf.write_all("foocontents".as_bytes())?;
}
assert!(dp.exists("a/.btmp.a")?);
assert!(dp.exists("a/foo")?);
assert!(dp.exists("a/.btmp.foo")?);
assert!(dp.exists("a/.btmp.a")?);
assert!(dp.exists(".btmp.b/b")?);
assert!(dp.exists(".btmp.b/foo")?);
cleanup_tmp(&dp)?;
assert!(!dp.exists("a/.btmp.a")?);
assert!(dp.exists("a/foo")?);
assert!(!dp.exists("a/.btmp.foo")?);
assert!(!dp.exists(".btmp.b")?);
Ok(())
}
#[test]
fn test_apply_with_file() -> Result<()> {
let tmpd = tempfile::tempdir()?;
let p = tmpd.path();
let pa = p.join("a");
let pb = p.join("b");
std::fs::create_dir(&pa)?;
std::fs::create_dir(&pb)?;
let a = openat::Dir::open(&pa)?;
let b = openat::Dir::open(&pb)?;
a.create_dir("foo", 0o755)?;
a.create_dir("bar", 0o755)?;
let foo = Path::new("foo/bar");
let bar = Path::new("bar/foo");
let testfile = "testfile";
{
let mut buf = a.write_file(foo, 0o644)?;
buf.write_all("foocontents".as_bytes())?;
let mut buf = a.write_file(bar, 0o644)?;
buf.write_all("barcontents".as_bytes())?;
let mut buf = a.write_file(testfile, 0o644)?;
buf.write_all("testfilecontents".as_bytes())?;
}

let diff = run_diff(&a, &b)?;
assert_eq!(diff.count(), 3);
b.create_dir("foo", 0o755)?;
{
let mut buf = b.write_file(foo, 0o644)?;
buf.write_all("foocontents".as_bytes())?;
}
let b_btime_foo = fs::metadata(pb.join(foo))?.created()?;

{
let diff = run_diff(&b, &a)?;
assert_eq!(diff.count(), 2);
apply_diff(&a, &b, &diff, None).context("test additional files")?;
assert_eq!(
String::from_utf8(std::fs::read(pb.join(testfile))?)?,
"testfilecontents"
);
assert_eq!(
String::from_utf8(std::fs::read(pb.join(bar))?)?,
"barcontents"
);
// creation time is not changed for unchanged file
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
assert_eq!(b_btime_foo_new, b_btime_foo);
}
{
fs::write(pa.join(testfile), "newtestfile")?;
fs::write(pa.join(bar), "newbar")?;
let diff = run_diff(&b, &a)?;
assert_eq!(diff.count(), 2);
apply_diff(&a, &b, &diff, None).context("test changed files")?;
assert_eq!(
String::from_utf8(std::fs::read(pb.join(testfile))?)?,
"newtestfile"
);
assert_eq!(String::from_utf8(std::fs::read(pb.join(bar))?)?, "newbar");
// creation time is not changed for unchanged file
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
assert_eq!(b_btime_foo_new, b_btime_foo);
}
{
a.remove_file(testfile)?;
a.remove_file(bar)?;
let diff = run_diff(&b, &a)?;
assert_eq!(diff.count(), 2);
apply_diff(&a, &b, &diff, None).context("test removed files")?;
assert_eq!(b.exists(testfile)?, false);
assert_eq!(b.exists(bar)?, false);
let diff = run_diff(&b, &a)?;
assert_eq!(diff.count(), 0);
// creation time is not changed for unchanged file
let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
assert_eq!(b_btime_foo_new, b_btime_foo);
}

Ok(())
}
}

0 comments on commit 7d124de

Please sign in to comment.