Repository: firecracker-microvm/firecracker
File: src/jailer/src/env.rs (Rust)

Code before:
            .ok_or_else(|| JailerError::ExtractFileName(self.exec_file_path.clone()))?;
        let jailer_exec_file_path = self.chroot_dir.join(exec_file_name);

        // We do a copy instead of a hard-link for 2 reasons
        // 1. hard-linking is not possible if the file is in another device
        // 2. while hardlinking would save up disk space and also memory by sharing parts of the
        //    Firecracker binary (like the executable .text section), this latter part is not
        //    desirable in Firecracker's threat model. Copying prevents 2 Firecracker processes from
        //    sharing memory.
        let mut src_file = OpenOptions::new()
            .read(true)
            .open(&self.exec_file_path)
            .map_err(|err| JailerError::Open(self.exec_file_path.clone(), err))?;
        let mut dst_file = OpenOptions::new()
            .write(true)
            .create(true)
            // Don't allow symlinks
            .custom_flags(libc::O_NOFOLLOW)
            // The file only needs to be executable by the owner
            .mode(0o100)
            .open(&jailer_exec_file_path)
            .map_err(|err| JailerError::Open(jailer_exec_file_path.clone(), err))?;

        // Mark destination file as owned by the specidief uid/gid
        let c_path = CString::new(jailer_exec_file_path.to_str().unwrap()).unwrap();
        // SAFETY: This is safe because folder was checked for a null-terminator.
        SyscallReturnCode(unsafe { libc::chown(c_path.as_ptr(), self.uid(), self.gid()) })
            .into_empty_result()
            .map_err(|err| JailerError::ChangeFileOwner(jailer_exec_file_path.clone(), err))?;

        // Ignore the output since it is not interesting in this case
        _ = std::io::copy(&mut src_file, &mut dst_file).map_err(|err| {
            JailerError::Copy(
                self.exec_file_path.clone(),
                jailer_exec_file_path.clone(),
                err,
            )
        })?;

        Ok(exec_file_name.to_owned())
    }

    fn join_netns(path: &str) -> Result<(), JailerError> {
        // The fd backing the file will be automatically dropped at the end of the scope
        let netns =
            File::open(path).map_err(|err| JailerError::FileOpen(PathBuf::from(path), err))?;

        // SAFETY: Safe because we are passing valid parameters.
        SyscallReturnCode(unsafe { libc::setns(netns.as_raw_fd(), libc::CLONE_NEWNET) })
            .into_empty_result()
            .map_err(JailerError::SetNetNs)

Review comment (suggestion):
I don't think it's just the folder that have nul-terminator checked. `c_path` (i.e. the entire path including the folder and the filename) is `CString` that ensures it's C-compatible (i.e. nul-terminated string with no null bytes in the middle). In addition, I think what we really have to do is the given pointer's lifetime is valid, although it's almost self-evident.
```suggestion
        // SAFETY: `c_path` is a C-compatible string (i.e. `CString`) and lives across the call.
```

Code after:
            .ok_or_else(|| JailerError::ExtractFileName(self.exec_file_path.clone()))?;
        let jailer_exec_file_path = self.chroot_dir.join(exec_file_name);

        // We do a copy instead of a hard-link for 2 reasons
        // 1. hard-linking is not possible if the file is in another device
        // 2. while hardlinking would save up disk space and also memory by sharing parts of the
        //    Firecracker binary (like the executable .text section), this latter part is not
        //    desirable in Firecracker's threat model. Copying prevents 2 Firecracker processes from
        //    sharing memory.
        let mut src_file = OpenOptions::new()
            .read(true)
            .open(&self.exec_file_path)
            .map_err(|err| JailerError::Open(self.exec_file_path.clone(), err))?;
        let src_file_metadata = src_file
            .metadata()
            .map_err(|err| JailerError::Metadata(self.exec_file_path.clone(), err))?;
        let src_file_mode = src_file_metadata.mode();
        let mut dst_file = OpenOptions::new()
            .write(true)
            .create(true)
            // Don't allow symlinks
            .custom_flags(libc::O_NOFOLLOW)
            .mode(src_file_mode)
            .open(&jailer_exec_file_path)
            .map_err(|err| JailerError::Open(jailer_exec_file_path.clone(), err))?;
        let dst_file_metadata = dst_file
            .metadata()
            .map_err(|err| JailerError::Metadata(jailer_exec_file_path.clone(), err))?;
        if 1 < dst_file_metadata.nlink() {
            return Err(JailerError::HardLink(jailer_exec_file_path.clone()));
        }

        // Mark destination file as owned by the specified uid/gid
        fchown(&dst_file, Some(self.uid()), Some(self.gid()))
            .map_err(|err| JailerError::ChangeFileOwner(jailer_exec_file_path.clone(), err))?;

        // Ignore the output since it is not interesting in this case
        _ = std::io::copy(&mut src_file, &mut dst_file).map_err(|err| {
            JailerError::Copy(
                self.exec_file_path.clone(),
                jailer_exec_file_path.clone(),
                err,
            )
        })?;

        Ok(exec_file_name.to_owned())
    }

    fn join_netns(path: &str) -> Result<(), JailerError> {
        // The fd backing the file will be automatically dropped at the end of the scope
        let netns =