Skip to content

Commit 70c2b77

Browse files
committed
blockdev: Fix loopback device resource leak on signal interruption
This commit implements creating a signal-safe cleanup helper for loopback devices to prevent resource leaks when bootc install --via-loopback is interrupted by signals like SIGINT (Ctrl-C). The solution uses an 'out-of-process drop' helper that: - Forks a cleanup helper process when creating a loopback device - Uses PR_SET_PDEATHSIG to detect when the parent process dies - Masks most signals to avoid being killed accidentally - Automatically cleans up leaked loopback devices if the parent dies - Gracefully terminates when the parent performs normal cleanup This prevents the common issue where interrupting bootc install --via-loopback with Ctrl-C would leave /dev/loopN devices allocated on the system.
1 parent c76d9e2 commit 70c2b77

File tree

3 files changed

+241
-1
lines changed

3 files changed

+241
-1
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

blockdev/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@ anyhow = { workspace = true }
1313
bootc-utils = { path = "../utils" }
1414
camino = { workspace = true, features = ["serde1"] }
1515
fn-error-context = { workspace = true }
16+
libc = { workspace = true }
1617
regex = "1.10.4"
1718
serde = { workspace = true, features = ["derive"] }
1819
serde_json = { workspace = true }
1920
tracing = { workspace = true }
2021

2122
[dev-dependencies]
2223
indoc = "2.0.5"
24+
tempfile = { workspace = true }
2325

2426
[lib]
2527
path = "src/blockdev.rs"

blockdev/src/blockdev.rs

Lines changed: 237 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,14 @@ pub fn partitions_of(dev: &Utf8Path) -> Result<PartitionTable> {
181181

182182
pub struct LoopbackDevice {
183183
pub dev: Option<Utf8PathBuf>,
184+
// Handle to the cleanup helper process
185+
cleanup_handle: Option<LoopbackCleanupHandle>,
186+
}
187+
188+
/// Handle to manage the cleanup helper process for loopback devices
189+
struct LoopbackCleanupHandle {
190+
/// Process ID of the cleanup helper
191+
helper_pid: u32,
184192
}
185193

186194
impl LoopbackDevice {
@@ -208,7 +216,19 @@ impl LoopbackDevice {
208216
.run_get_string()?;
209217
let dev = Utf8PathBuf::from(dev.trim());
210218
tracing::debug!("Allocated loopback {dev}");
211-
Ok(Self { dev: Some(dev) })
219+
220+
// Try to spawn cleanup helper process - if it fails, continue without it
221+
let cleanup_handle = Self::spawn_cleanup_helper(dev.as_str())
222+
.map_err(|e| {
223+
tracing::warn!("Failed to spawn loopback cleanup helper: {}, continuing without signal protection", e);
224+
e
225+
})
226+
.ok();
227+
228+
Ok(Self {
229+
dev: Some(dev),
230+
cleanup_handle,
231+
})
212232
}
213233

214234
// Access the path to the loopback block device.
@@ -217,13 +237,173 @@ impl LoopbackDevice {
217237
self.dev.as_deref().unwrap()
218238
}
219239

240+
/// Spawn a cleanup helper process that will clean up the loopback device
241+
/// if the parent process dies unexpectedly
242+
fn spawn_cleanup_helper(device_path: &str) -> Result<LoopbackCleanupHandle> {
243+
let device_path = device_path.to_string();
244+
245+
// Fork the cleanup helper process
246+
match unsafe { libc::fork() } {
247+
-1 => anyhow::bail!("Failed to fork cleanup helper process"),
248+
0 => {
249+
// Child process - this will be the cleanup helper
250+
// This function will not return
251+
Self::cleanup_helper_main(device_path);
252+
}
253+
child_pid => {
254+
// Parent process
255+
Ok(LoopbackCleanupHandle {
256+
helper_pid: child_pid as u32,
257+
})
258+
}
259+
}
260+
}
261+
262+
/// Main function for the cleanup helper process
263+
/// This function does not return - it either exits normally or via exec
264+
fn cleanup_helper_main(device_path: String) -> ! {
265+
// Close stdin, stdout, stderr and other inherited file descriptors
266+
unsafe {
267+
for fd in 0..=2 {
268+
libc::close(fd);
269+
}
270+
// Redirect to /dev/null in case something tries to write
271+
let null_fd = libc::open(b"/dev/null\0".as_ptr() as *const i8, libc::O_RDWR);
272+
if null_fd >= 0 {
273+
libc::dup2(null_fd, 0);
274+
libc::dup2(null_fd, 1);
275+
libc::dup2(null_fd, 2);
276+
if null_fd > 2 {
277+
libc::close(null_fd);
278+
}
279+
}
280+
}
281+
282+
// Set up death signal notification - we want to be notified when parent dies
283+
unsafe {
284+
if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGUSR1) != 0 {
285+
std::process::exit(1);
286+
}
287+
}
288+
289+
// Mask most signals to avoid being killed accidentally
290+
// But leave SIGUSR1 unmasked so we can receive the death notification
291+
unsafe {
292+
let mut sigset: libc::sigset_t = std::mem::zeroed();
293+
libc::sigfillset(&mut sigset);
294+
// Don't mask SIGKILL, SIGSTOP (can't be masked anyway), or our death signal
295+
libc::sigdelset(&mut sigset, libc::SIGKILL);
296+
libc::sigdelset(&mut sigset, libc::SIGSTOP);
297+
libc::sigdelset(&mut sigset, libc::SIGUSR1); // We'll use SIGUSR1 as our death signal
298+
299+
if libc::pthread_sigmask(libc::SIG_SETMASK, &sigset, std::ptr::null_mut()) != 0 {
300+
let err = std::io::Error::last_os_error();
301+
tracing::error!("pthread_sigmask failed: {}", err);
302+
std::process::exit(1);
303+
}
304+
}
305+
306+
// Wait for death signal or normal termination
307+
let mut siginfo: libc::siginfo_t = unsafe { std::mem::zeroed() };
308+
let sigset = {
309+
let mut sigset: libc::sigset_t = unsafe { std::mem::zeroed() };
310+
unsafe {
311+
libc::sigemptyset(&mut sigset);
312+
libc::sigaddset(&mut sigset, libc::SIGUSR1);
313+
libc::sigaddset(&mut sigset, libc::SIGTERM); // Also listen for SIGTERM (normal cleanup)
314+
}
315+
sigset
316+
};
317+
318+
// Wait for a signal
319+
let result = unsafe {
320+
let result = libc::sigwaitinfo(&sigset, &mut siginfo);
321+
if result == -1 {
322+
let err = std::io::Error::last_os_error();
323+
tracing::error!("sigwaitinfo failed: {}", err);
324+
std::process::exit(1);
325+
}
326+
result
327+
};
328+
329+
if result > 0 {
330+
if siginfo.si_signo == libc::SIGUSR1 {
331+
// Parent died unexpectedly, clean up the loopback device
332+
let status = std::process::Command::new("losetup")
333+
.args(["-d", &device_path])
334+
.status();
335+
336+
match status {
337+
Ok(exit_status) if exit_status.success() => {
338+
// Write to stderr since we closed stdout
339+
let _ = std::io::Write::write_all(
340+
&mut std::io::stderr(),
341+
format!("bootc: cleaned up leaked loopback device {}\n", device_path)
342+
.as_bytes(),
343+
);
344+
std::process::exit(0);
345+
}
346+
Ok(_) => {
347+
let _ = std::io::Write::write_all(
348+
&mut std::io::stderr(),
349+
format!(
350+
"bootc: failed to clean up loopback device {}\n",
351+
device_path
352+
)
353+
.as_bytes(),
354+
);
355+
std::process::exit(1);
356+
}
357+
Err(e) => {
358+
let _ = std::io::Write::write_all(
359+
&mut std::io::stderr(),
360+
format!(
361+
"bootc: error cleaning up loopback device {}: {}\n",
362+
device_path, e
363+
)
364+
.as_bytes(),
365+
);
366+
std::process::exit(1);
367+
}
368+
}
369+
} else if siginfo.si_signo == libc::SIGTERM {
370+
// Normal cleanup signal from parent
371+
std::process::exit(0);
372+
}
373+
}
374+
375+
// If we get here, something went wrong
376+
std::process::exit(1);
377+
}
378+
220379
// Shared backend for our `close` and `drop` implementations.
221380
fn impl_close(&mut self) -> Result<()> {
222381
// SAFETY: This is the only place we take the option
223382
let Some(dev) = self.dev.take() else {
224383
tracing::trace!("loopback device already deallocated");
225384
return Ok(());
226385
};
386+
387+
// Kill the cleanup helper since we're cleaning up normally
388+
if let Some(cleanup_handle) = self.cleanup_handle.take() {
389+
// Kill the helper process since we're doing normal cleanup
390+
unsafe {
391+
if libc::kill(cleanup_handle.helper_pid as i32, libc::SIGTERM) != 0 {
392+
let err = std::io::Error::last_os_error();
393+
tracing::warn!("kill failed: {}", err);
394+
}
395+
}
396+
// Wait for it to exit (non-blocking)
397+
unsafe {
398+
let mut status = 0;
399+
if libc::waitpid(cleanup_handle.helper_pid as i32, &mut status, libc::WNOHANG) == -1
400+
{
401+
let err = std::io::Error::last_os_error();
402+
tracing::warn!("waitpid failed: {}", err);
403+
}
404+
}
405+
}
406+
227407
Command::new("losetup").args(["-d", dev.as_str()]).run()
228408
}
229409

@@ -389,4 +569,60 @@ mod test {
389569
);
390570
Ok(())
391571
}
572+
573+
#[test]
574+
fn test_loopback_device_with_cleanup_helper() -> Result<()> {
575+
// Only run this test if we have permissions and losetup is available
576+
if !std::path::Path::new("/usr/bin/losetup").exists()
577+
&& !std::path::Path::new("/sbin/losetup").exists()
578+
{
579+
eprintln!("Skipping loopback test: losetup not found");
580+
return Ok(());
581+
}
582+
583+
// Check if we can run as root or have the necessary capabilities
584+
if unsafe { libc::geteuid() } != 0 {
585+
eprintln!("Skipping loopback test: requires root privileges");
586+
return Ok(());
587+
}
588+
589+
// Create a temporary file to use as the loopback backing store
590+
let mut temp_file = tempfile::NamedTempFile::new()?;
591+
592+
// Write some data to make it a valid file
593+
{
594+
use std::io::Write;
595+
// Create a 10MB file
596+
let data = vec![0u8; 10 * 1024 * 1024];
597+
temp_file.write_all(&data)?;
598+
temp_file.flush()?;
599+
}
600+
601+
// Convert to TempPath so we control when it gets deleted
602+
let temp_path = temp_file.into_temp_path();
603+
604+
// Test creating and cleaning up a loopback device
605+
let loopback = LoopbackDevice::new(&temp_path)?;
606+
let device_path = loopback.path().to_string();
607+
608+
// Verify the device was created
609+
assert!(device_path.starts_with("/dev/loop"));
610+
assert!(std::path::Path::new(&device_path).exists());
611+
612+
// Verify we have a cleanup handle
613+
assert!(
614+
loopback.cleanup_handle.is_some(),
615+
"Cleanup helper should be spawned"
616+
);
617+
618+
// Explicitly close the loopback device to test cleanup
619+
loopback.close()?;
620+
621+
// Give a moment for cleanup to happen
622+
std::thread::sleep(std::time::Duration::from_millis(100));
623+
624+
// TempPath will be automatically deleted when dropped at end of scope
625+
626+
Ok(())
627+
}
392628
}

0 commit comments

Comments
 (0)