fix(jailer): harden aarch64 cache-info file ownership

CHANGELOG.md

@@ -26,6 +26,9 @@ and this project adheres to
   restore, triggered by taking a snapshot with a TX descriptor in-flight. On
   restore the device now replays the TX queue notification so in-flight TX
   descriptors are re-processed and notification suppression is re-armed.
+- [#5956](https://github.com/firecracker-microvm/firecracker/pull/5956): Fixed a
+  TOCTOU race in the aarch64 jailer when setting ownership of the CPU cache and
+  `MIDR_EL1` information files copied into the chroot.
 
 ## [1.16.0]
 

src/jailer/src/env.rs

@@ -133,6 +133,28 @@ pub struct Env {
     uffd_dev_minor: Option<u32>,
 }
 
+/// Creates a new file owned by the given uid/gid at `dst` and writes `line`
+/// into it, plus a trailing newline.
+#[cfg(target_arch = "aarch64")]
+fn create_owned_file_with_data(
+    dst: &Path,
+    line: String,
+    uid: u32,
+    gid: u32,
+) -> Result<(), JailerError> {
+    let mut file = OpenOptions::new()
+        .write(true)
+        // `create_new` maps to `O_CREAT | O_EXCL`: fail if `dst` already exists.
+        .create_new(true)
+        .custom_flags(libc::O_NOFOLLOW)
+        .open(dst)
+        .map_err(|err| JailerError::Write(dst.to_path_buf(), err))?;
+    file.write_all(format!("{}\n", line).as_bytes())
+        .map_err(|err| JailerError::Write(dst.to_path_buf(), err))?;
+    fchown(&file, Some(uid), Some(gid))
+        .map_err(|err| JailerError::ChangeFileOwner(dst.to_path_buf(), err))
+}
+
 impl Env {
     pub fn new(
         arguments: &arg_parser::Arguments,
@@ -548,7 +570,7 @@ impl Env {
 
     #[cfg(target_arch = "aarch64")]
     fn copy_cache_info(&self) -> Result<(), JailerError> {
-        use crate::{readln_special, to_cstring, writeln_special};
+        use crate::readln_special;
 
         const HOST_CACHE_INFO: &str = "/sys/devices/system/cpu/cpu0/cache";
         // Based on https://elixir.free-electrons.com/linux/v4.9.62/source/arch/arm64/kernel/cacheinfo.c#L29.
@@ -592,18 +614,7 @@ impl Env {
                 let jailer_cache_file = jailer_path.join(entry);
 
                 if let Ok(line) = readln_special(&host_cache_file) {
-                    writeln_special(&jailer_cache_file, line)?;
-
-                    // We now change the permissions.
-                    let dest_path_cstr = to_cstring(&jailer_cache_file)?;
-                    // SAFETY: Safe because dest_path_cstr is null-terminated.
-                    SyscallReturnCode(unsafe {
-                        libc::chown(dest_path_cstr.as_ptr(), self.uid(), self.gid())
-                    })
-                    .into_empty_result()
-                    .map_err(|err| {
-                        JailerError::ChangeFileOwner(jailer_cache_file.to_owned(), err)
-                    })?;
+                    create_owned_file_with_data(&jailer_cache_file, line, self.uid(), self.gid())?;
                 }
             }
         }
@@ -612,7 +623,7 @@ impl Env {
 
     #[cfg(target_arch = "aarch64")]
     fn copy_midr_el1_info(&self) -> Result<(), JailerError> {
-        use crate::{readln_special, to_cstring, writeln_special};
+        use crate::readln_special;
 
         const HOST_MIDR_EL1_INFO: &str = "/sys/devices/system/cpu/cpu0/regs/identification";
 
@@ -624,16 +635,10 @@ impl Env {
         let host_midr_el1_file = PathBuf::from(format!("{}/midr_el1", HOST_MIDR_EL1_INFO));
         let jailer_midr_el1_file = jailer_midr_el1_directory.join("midr_el1");
 
-        // Read and copy the MIDR_EL1 file to Jailer
+        // Read the host MIDR_EL1 value and write it into the jail, owned by
+        // the jailed uid:gid.
         let line = readln_special(&host_midr_el1_file)?;
-        writeln_special(&jailer_midr_el1_file, line)?;
-
-        // Change the permissions.
-        let dest_path_cstr = to_cstring(&jailer_midr_el1_file)?;
-        // SAFETY: Safe because `dest_path_cstr` is null-terminated.
-        SyscallReturnCode(unsafe { libc::chown(dest_path_cstr.as_ptr(), self.uid(), self.gid()) })
-            .into_empty_result()
-            .map_err(|err| JailerError::ChangeFileOwner(jailer_midr_el1_file.to_owned(), err))?;
+        create_owned_file_with_data(&jailer_midr_el1_file, line, self.uid(), self.gid())?;
 
         Ok(())
     }
@@ -1428,6 +1433,10 @@ mod tests {
 
         let env = create_env(&mock_cgroups.proc_mounts_path);
 
+        // Clear any leftovers from a previous run of this test, as
+        // `copy_cache_info` expects to be creating these files.
+        let _ = fs::remove_dir_all(env.chroot_dir());
+
         // Create the required chroot dir hierarchy.
         fs::create_dir_all(env.chroot_dir()).expect("Could not create dir hierarchy.");