From d5bae4fe1ddc334e74dd873b5312929c2b792649 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 11 Jun 2026 10:45:57 +0530 Subject: [PATCH] Do not clean tempdir if we fail to unmount Ran into this issue by chance in bootupd where the following drop ordering ```rust drop(tempdir) drop(mount) // unmounts thing mounted at tempdir ``` was causing all the contents of the mounted device to be deleted because the tempdir was being deleted. We might run into the same issue with our Tempdir impl if we fail to unount the ESP and tempdir is dropped deleting everything in the ESP. To prevent that, we now have a separate struct `MountpointTempdir` which creates the tempdir for us, explicitly sets `disable_cleanup` to false so that the dir isn't cleaned up on Drop. On drop, we clean it up ourselves intentionally using `remove_dir` and not `remove_dir_all` to make sure we don't end up deleting stuff in the dir. Signed-off-by: Pragyan Poudyal --- crates/mount/src/tempmount.rs | 42 ++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/mount/src/tempmount.rs b/crates/mount/src/tempmount.rs index afd5dd527..48d28f2c2 100644 --- a/crates/mount/src/tempmount.rs +++ b/crates/mount/src/tempmount.rs @@ -49,11 +49,43 @@ impl Drop for MountGuard { } } +/// Holds a tempdir with custom Drop impl +#[derive(Debug)] +pub struct MountpointTempdir(tempfile::TempDir); + +impl std::ops::Deref for MountpointTempdir { + type Target = tempfile::TempDir; + fn deref(&self) -> &tempfile::TempDir { + &self.0 + } +} + +impl MountpointTempdir { + fn new() -> Result { + let mut tmpdir = tempfile::TempDir::new()?; + tmpdir.disable_cleanup(true); // We will clean this ourselves + Ok(Self(tmpdir)) + } +} + +impl Drop for MountpointTempdir { + fn drop(&mut self) { + // Intentionally not using remove_dir_all so that we don't + // accidentally end up deleting anything mounted at this path + if let Err(e) = std::fs::remove_dir(self.path()) { + tracing::warn!( + "Failed to remove tmpdir at {}: {e:?}", + self.path().display() + ) + } + } +} + /// RAII wrapper for a temporary mount that is automatically unmounted on drop. #[derive(Debug)] pub struct TempMount { /// The backing temporary directory. - pub dir: tempfile::TempDir, + pub dir: MountpointTempdir, /// An open handle to the mounted directory. pub fd: Dir, } @@ -67,7 +99,7 @@ impl TempMount { flags: MountFlags, data: Option<&std::ffi::CStr>, ) -> Result { - let tempdir = tempfile::TempDir::new()?; + let tempdir = MountpointTempdir::new()?; let utf8path = Utf8Path::from_path(tempdir.path()) .ok_or(anyhow::anyhow!("Failed to convert path to UTF-8 Path"))?; @@ -81,7 +113,7 @@ impl TempMount { Ok(fd) => fd, Err(e) => { unmount(tempdir.path(), UnmountFlags::DETACH)?; - Err(e)? + return Err(e)?; } }; @@ -91,7 +123,7 @@ impl TempMount { /// Mount and fd acquired with `open_tree` like syscall #[context("Mounting fd")] pub fn mount_fd(mnt_fd: impl AsFd) -> Result { - let tempdir = tempfile::TempDir::new()?; + let tempdir = MountpointTempdir::new()?; move_mount( mnt_fd.as_fd(), @@ -109,7 +141,7 @@ impl TempMount { Ok(fd) => fd, Err(e) => { unmount(tempdir.path(), UnmountFlags::DETACH)?; - Err(e)? + return Err(e)?; } };