Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install to-disk: Check for mounts #720

Closed
cgwalters opened this issue Jul 23, 2024 · 0 comments · Fixed by #805
Closed

install to-disk: Check for mounts #720

cgwalters opened this issue Jul 23, 2024 · 0 comments · Fixed by #805
Assignees
Labels
area/install Issues related to `bootc install` enhancement New feature or request good first issue Good for newcomers

Comments

@cgwalters
Copy link
Collaborator

cgwalters commented Jul 23, 2024

Split from #718 (review)

we definitely need to strengthen the to-disk path to bail hard if the device is mounted for example, even if --wipe is specified.

I looked at implementing this like the below, but it doesn't work because our mount namespace won't have the physical devices mounted.

Instead, I think we can implement this by using findmnt -N 1 --source /path/to/dev for each device...or just scraping the overall findmnt -N 1 and matching up the devices on our own.

From c86a452b05a7b94c341269e147bc31d4c0a82feb Mon Sep 17 00:00:00 2001
From: Colin Walters <[email protected]>
Date: Tue, 23 Jul 2024 12:34:55 -0400
Subject: [PATCH] wip

Signed-off-by: Colin Walters <[email protected]>
---
 lib/src/blockdev.rs         | 17 +++++++++++++++++
 lib/src/install/baseline.rs |  4 ++++
 2 files changed, 21 insertions(+)

diff --git a/lib/src/blockdev.rs b/lib/src/blockdev.rs
index 3c69080..f18e187 100644
--- a/lib/src/blockdev.rs
+++ b/lib/src/blockdev.rs
@@ -36,6 +36,9 @@ pub(crate) struct Device {
     // Filesystem-related properties
     pub(crate) label: Option<String>,
     pub(crate) fstype: Option<String>,
+
+    // VFS properties
+    pub(crate) mountpoints: Option<Vec<String>>,
 }
 
 impl Device {
@@ -49,6 +52,20 @@ impl Device {
         self.children.as_ref().map_or(false, |v| !v.is_empty())
     }
 
+    /// Return the first mounted device (this device, or any of its children) along
+    /// with its mountpoints.
+    pub(crate) fn find_first_mounted(&self) -> Option<(&Device, &[String])> {
+        match self.mountpoints.as_deref() {
+            Some(mounts @ [_, ..]) => return Some((self, mounts)),
+            Some([]) | None => {}
+        }
+        self.children
+            .iter()
+            .flatten()
+            .filter_map(|c| c.find_first_mounted())
+            .next()
+    }
+
     // The "start" parameter was only added in a version of util-linux that's only
     // in Fedora 40 as of this writing.
     fn backfill_start(&mut self) -> Result<()> {
diff --git a/lib/src/install/baseline.rs b/lib/src/install/baseline.rs
index 180a3a1..fb95731 100644
--- a/lib/src/install/baseline.rs
+++ b/lib/src/install/baseline.rs
@@ -191,6 +191,10 @@ pub(crate) fn install_create_rootfs(
             opts.device
         );
     }
+    // Always disallow writing to a mounted device
+    if let Some((dev, mounts)) = device.find_first_mounted() {
+        anyhow::bail!("Device {} is mounted: {:?}", dev.path(), mounts)
+    }
 
     let run_bootc = Utf8Path::new(RUN_BOOTC);
     let mntdir = run_bootc.join("mounts");
-- 
2.44.0
@cgwalters cgwalters added enhancement New feature or request good first issue Good for newcomers area/install Issues related to `bootc install` labels Jul 23, 2024
@djach7 djach7 self-assigned this Sep 19, 2024
cgwalters pushed a commit to cgwalters/bootc that referenced this issue Nov 5, 2024
…-0.38.36

build(deps): bump rustix from 0.38.35 to 0.38.36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install` enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants