From 5f28ae75ea350ef8fc53ffeac357a6338b1d828a Mon Sep 17 00:00:00 2001 From: nikstur Date: Sat, 28 Jan 2023 03:17:35 +0100 Subject: [PATCH] tool: atomically write to ESP To minimize the potential for irrecoverable errors, only atomic writes to the ESP are performed. This is implemented by first copying the file to the destination with a `.tmp` suffix and then renaming it to the final desintation. This is atomic because the rename operation is atomic on POSIX platforms. Specifically, this means that even if the system crashes during the operation, the final desintation path will most likely be intact if it exists at all. There are some nuances to this however. It **cannot** be actually guaranteed that the operation was performed on the filesystem level. However, this is the best we can do for now. For reference: - POSIX rename(2): https://pubs.opengroup.org/onlinepubs/9699919799/ - Rust fs::rename corresponds to rename(2) on Unix: https://doc.rust-lang.org/std/fs/fn.rename.html - Rust fs::rename is implemented using libc's rename: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/fs.rs#L1397 - Renaming in libc is atomic: https://www.gnu.org/software/libc/manual/html_node/Renaming-Files.html --- rust/tool/src/install.rs | 59 ++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/rust/tool/src/install.rs b/rust/tool/src/install.rs index 7a0b063..51dd8ae 100644 --- a/rust/tool/src/install.rs +++ b/rust/tool/src/install.rs @@ -143,7 +143,7 @@ impl Installer { println!("Appending secrets to initrd..."); let initrd_location = tempdir.path().join("initrd"); - copy( + fs::copy( bootspec .initrd .as_ref() @@ -241,25 +241,39 @@ fn install_signed(key_pair: &KeyPair, from: &Path, to: &Path) -> Result<()> { /// Sign and forcibly install a PE file. /// /// If the file already exists at the destination, it is overwritten. +/// +/// This is implemented as an atomic write. The file is first written to the destination with a +/// `.tmp` suffix and then renamed to its final name. This is atomic, because a rename is an atomic +/// operation on POSIX platforms. fn force_install_signed(key_pair: &KeyPair, from: &Path, to: &Path) -> Result<()> { println!("Signing and installing {}...", to.display()); - ensure_parent_dir(to); + let to_tmp = to.with_extension(".tmp"); + ensure_parent_dir(&to_tmp); key_pair - .sign_and_copy(from, to) + .sign_and_copy(from, &to_tmp) .with_context(|| format!("Failed to copy and sign file from {from:?} to {to:?}"))?; + fs::rename(&to_tmp, to).with_context(|| { + format!("Failed to move temporary file {to_tmp:?} to final location {to:?}") + })?; Ok(()) } -/// Install an arbitrary file +/// Install an arbitrary file. /// -/// The file is only copied if it doesn't exist at the destination +/// The file is only copied if it doesn't exist at the destination. +/// +/// This function is only designed to copy files to the ESP. It sets the permission bits of the +/// file at the destination to 0o755, the expected permissions for a vfat ESP. This is useful for +/// producing file systems trees which can then be converted to a file system image. fn install(from: &Path, to: &Path) -> Result<()> { if to.exists() { println!("{} already exists, skipping...", to.display()); } else { println!("Installing {}...", to.display()); ensure_parent_dir(to); - copy(from, to)?; + atomic_copy(from, to)?; + set_permission_bits(to, 0o755) + .with_context(|| format!("Failed to set permission bits to 0o755 on file: {to:?}"))?; } Ok(()) @@ -293,20 +307,29 @@ fn assemble_kernel_cmdline(init: &Path, kernel_params: Vec) -> Vec Result<()> { - ensure_parent_dir(to); - fs::copy(from, to) - .with_context(|| format!("Failed to copy from {} to {}", from.display(), to.display()))?; +/// Atomically copy a file. +/// +/// The file is first written to the destination with a `.tmp` suffix and then renamed to its final +/// name. This is atomic, because a rename is an atomic operation on POSIX platforms. +fn atomic_copy(from: &Path, to: &Path) -> Result<()> { + let to_tmp = to.with_extension(".tmp"); - // Set permission of all files copied to 0o755 - let mut perms = fs::metadata(to) - .with_context(|| format!("File {} doesn't have metadata", to.display()))? + fs::copy(from, &to_tmp) + .with_context(|| format!("Failed to copy from {from:?} to {to_tmp:?}",))?; + + fs::rename(&to_tmp, to).with_context(|| { + format!("Failed to move temporary file {to_tmp:?} to final location {to:?}") + }) +} + +/// Set the octal permission bits of the specified file. +fn set_permission_bits(path: &Path, permission_bits: u32) -> Result<()> { + let mut perms = fs::metadata(path) + .with_context(|| format!("File {path:?} doesn't have any metadata"))? .permissions(); - perms.set_mode(0o755); - fs::set_permissions(to, perms) - .with_context(|| format!("Failed to set permissions to: {}", to.display()))?; - - Ok(()) + perms.set_mode(permission_bits); + fs::set_permissions(path, perms) + .with_context(|| format!("Failed to set permissions on {path:?}")) } // Ensures the parent directory of an arbitrary path exists