Skip to content

Commit 922eb71

Browse files
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 <pragyanpoudyal41999@gmail.com>
1 parent c818e03 commit 922eb71

1 file changed

Lines changed: 37 additions & 5 deletions

File tree

crates/mount/src/tempmount.rs

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,43 @@ impl Drop for MountGuard {
4949
}
5050
}
5151

52+
/// Holds a tempdir with custom Drop impl
53+
#[derive(Debug)]
54+
pub struct MountpointTempdir(tempfile::TempDir);
55+
56+
impl std::ops::Deref for MountpointTempdir {
57+
type Target = tempfile::TempDir;
58+
fn deref(&self) -> &tempfile::TempDir {
59+
&self.0
60+
}
61+
}
62+
63+
impl MountpointTempdir {
64+
fn new() -> Result<Self> {
65+
let mut tmpdir = tempfile::TempDir::new()?;
66+
tmpdir.disable_cleanup(true); // We will clean this ourselves
67+
Ok(Self(tmpdir))
68+
}
69+
}
70+
71+
impl Drop for MountpointTempdir {
72+
fn drop(&mut self) {
73+
// Intentionally not using remove_dir_all so that we don't
74+
// accidentally end up deleting anything mounted at this path
75+
if let Err(e) = std::fs::remove_dir(self.path()) {
76+
tracing::warn!(
77+
"Failed to remove tmpdir at {}: {e:?}",
78+
self.path().display()
79+
)
80+
}
81+
}
82+
}
83+
5284
/// RAII wrapper for a temporary mount that is automatically unmounted on drop.
5385
#[derive(Debug)]
5486
pub struct TempMount {
5587
/// The backing temporary directory.
56-
pub dir: tempfile::TempDir,
88+
pub dir: MountpointTempdir,
5789
/// An open handle to the mounted directory.
5890
pub fd: Dir,
5991
}
@@ -67,7 +99,7 @@ impl TempMount {
6799
flags: MountFlags,
68100
data: Option<&std::ffi::CStr>,
69101
) -> Result<Self> {
70-
let tempdir = tempfile::TempDir::new()?;
102+
let tempdir = MountpointTempdir::new()?;
71103

72104
let utf8path = Utf8Path::from_path(tempdir.path())
73105
.ok_or(anyhow::anyhow!("Failed to convert path to UTF-8 Path"))?;
@@ -81,7 +113,7 @@ impl TempMount {
81113
Ok(fd) => fd,
82114
Err(e) => {
83115
unmount(tempdir.path(), UnmountFlags::DETACH)?;
84-
Err(e)?
116+
return Err(e)?;
85117
}
86118
};
87119

@@ -91,7 +123,7 @@ impl TempMount {
91123
/// Mount and fd acquired with `open_tree` like syscall
92124
#[context("Mounting fd")]
93125
pub fn mount_fd(mnt_fd: impl AsFd) -> Result<Self> {
94-
let tempdir = tempfile::TempDir::new()?;
126+
let tempdir = MountpointTempdir::new()?;
95127

96128
move_mount(
97129
mnt_fd.as_fd(),
@@ -109,7 +141,7 @@ impl TempMount {
109141
Ok(fd) => fd,
110142
Err(e) => {
111143
unmount(tempdir.path(), UnmountFlags::DETACH)?;
112-
Err(e)?
144+
return Err(e)?;
113145
}
114146
};
115147

0 commit comments

Comments
 (0)