composefs: Find GRUB's boot directory on a separate /boot when upgrading

crates/lib/src/bootc_composefs/boot.rs

@@ -118,7 +118,7 @@ use crate::{
 };
 use crate::{parsers::grub_menuconfig::MenuEntry, store::BootedComposefs};
 
-use crate::install::{RootSetup, State};
+use crate::install::{BOOT, RootSetup, State};
 
 /// Contains the EFP's filesystem UUID. Used by grub
 pub(crate) const EFI_UUID_FILE: &str = "efiuuid.cfg";
@@ -304,7 +304,7 @@ fn read_dumpfile_from_fs(
     let root = fs.as_dir();
     let dumpfile_os = std::ffi::OsStr::new(dumpfile_name);
 
-    if let Ok(boot_dir) = root.get_directory_ref("boot".as_ref()) {
+    if let Ok(boot_dir) = root.get_directory_ref(BOOT.as_ref()) {
         if let Ok(file) = boot_dir.get_file(dumpfile_os) {
             return read_regular_file(file, repo);
         }
@@ -320,6 +320,39 @@ pub(crate) enum BootSetupType<'a> {
     Upgrade((&'a Storage, &'a BootedComposefs, &'a Host)),
 }
 
+impl BootSetupType<'_> {
+    /// The directory holding GRUB's configuration and the BLS entries: `boot`
+    /// in the target root at install time. On a booted system, it's
+    /// `/sysroot/boot`, or `/boot` if that's a separate boot partition.
+    fn grub_boot_path(&self) -> Result<Utf8PathBuf> {
+        match self {
+            Self::Setup((root_setup, ..)) => Ok(root_setup.physical_root_path.join(BOOT)),
+            Self::Upgrade((storage, ..)) => Ok(storage.require_grub_boot_path()?.to_owned()),
+        }
+    }
+}
+
+/// The absolute path GRUB resolves the kernels and initramfses in BLS entries
+/// against: the root of the filesystem holding `grub_boot_path`.
+fn grub_bls_abs_entries_path(grub_boot_path: &Utf8Path) -> Result<Utf8PathBuf> {
+    let (Some(parent), Some(name)) = (grub_boot_path.parent(), grub_boot_path.file_name()) else {
+        anyhow::bail!("Invalid boot path {grub_boot_path}");
+    };
+    let parent = Dir::open_ambient_dir(parent, ambient_authority())
+        .with_context(|| format!("Opening {parent}"))?;
+    // If "boot" is a partition, we want the paths to be absolute to "/"
+    let is_mountpoint = parent
+        .is_mountpoint(name)
+        .with_context(|| format!("Checking if {grub_boot_path} is a mountpoint"))?;
+    // We can be fairly sure that the kernels we target support `statx`
+    let root = Utf8Path::new("/");
+    Ok(if is_mountpoint == Some(true) {
+        root.to_owned()
+    } else {
+        root.join(BOOT)
+    })
+}
+
 #[derive(
     ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Default, JsonSchema,
 )]
@@ -744,7 +777,7 @@ pub(crate) fn setup_composefs_bls_boot(
 ) -> Result<String> {
     let id_hex = id.to_hex();
 
-    let (root_path, esp_device, mut cmdline_refs, bootloader) = match setup_type {
+    let (esp_device, mut cmdline_refs, bootloader) = match setup_type {
         BootSetupType::Setup((root_setup, state, postfetch, allow_missing_fsverity)) => {
             // root_setup.kargs has [root=UUID=<UUID>, "rw"]
             let mut cmdline_options = Cmdline::new();
@@ -781,7 +814,6 @@ pub(crate) fn setup_composefs_bls_boot(
             let esp_part = root_setup.device_info.find_first_colocated_esp()?;
 
             (
-                root_setup.physical_root_path.clone(),
                 esp_part.path(),
                 cmdline_options,
                 postfetch.detected_bootloader.clone(),
@@ -818,12 +850,7 @@ pub(crate) fn setup_composefs_bls_boot(
             let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?;
             let esp_dev = root_dev.find_first_colocated_esp()?;
 
-            (
-                Utf8PathBuf::from("/sysroot"),
-                esp_dev.path(),
-                cmdline,
-                bootloader,
-            )
+            (esp_dev.path(), cmdline, bootloader)
         }
     };
 
@@ -839,24 +866,17 @@ pub(crate) fn setup_composefs_bls_boot(
 
     let (entry_paths, _tmpdir_guard) = match bootloader.kind()? {
         BootloaderKind::GRUBClassic => {
-            let root = Dir::open_ambient_dir(&root_path, ambient_authority())
-                .context("Opening root path")?;
+            let boot_path = setup_type.grub_boot_path()?;
 
             // Grub wants the paths to be absolute against the mounted drive that the kernel +
             // initrd live in
-            //
-            // If "boot" is a partition, we want the paths to be absolute to "/"
-            let entries_path = match root.is_mountpoint("boot")? {
-                Some(true) => "/",
-                // We can be fairly sure that the kernels we target support `statx`
-                Some(false) | None => "/boot",
-            };
+            let abs_entries_path = grub_bls_abs_entries_path(&boot_path)?;
 
             (
                 BLSEntryPath {
-                    entries_path: root_path.join("boot"),
-                    config_path: root_path.join("boot"),
-                    abs_entries_path: entries_path.into(),
+                    entries_path: boot_path.clone(),
+                    config_path: boot_path,
+                    abs_entries_path,
                 },
                 None,
             )
@@ -1591,13 +1611,12 @@ fn resolve_boot_image_match(
 
 #[context("Writing Grub menuentry")]
 fn write_grub_uki_menuentry(
-    root_path: Utf8PathBuf,
     setup_type: &BootSetupType,
     boot_label: String,
     id: &Sha512HashValue,
     esp_device: &String,
 ) -> Result<()> {
-    let boot_dir = root_path.join("boot");
+    let boot_dir = setup_type.grub_boot_path()?;
     create_dir_all(&boot_dir).context("Failed to create boot dir")?;
 
     let is_upgrade = matches!(setup_type, BootSetupType::Upgrade(..));
@@ -1756,16 +1775,14 @@ pub(crate) fn setup_composefs_uki_boot(
     boot_ids: &ExpectedBootImageIds,
     entries: Vec<ComposefsBootEntry<Sha512HashValue>>,
 ) -> Result<(String, Sha512HashValue)> {
-    let (root_path, esp_device, bootloader, missing_fsverity_allowed, uki_addons) = match setup_type
-    {
+    let (esp_device, bootloader, missing_fsverity_allowed, uki_addons) = match setup_type {
         BootSetupType::Setup((root_setup, state, postfetch, allow_missing_fsverity)) => {
             state.require_no_kargs_for_uki()?;
 
             // Locate ESP partition device by walking up to the root disk(s)
             let esp_part = root_setup.device_info.find_first_colocated_esp()?;
 
             (
-                root_setup.physical_root_path.clone(),
                 esp_part.path(),
                 postfetch.detected_bootloader.clone(),
                 allow_missing_fsverity,
@@ -1774,15 +1791,13 @@ pub(crate) fn setup_composefs_uki_boot(
         }
 
         BootSetupType::Upgrade((storage, booted_cfs, host)) => {
-            let sysroot = Utf8PathBuf::from("/sysroot"); // Still needed for root_path
             let bootloader = host.require_composefs_booted()?.bootloader.clone();
 
             // Locate ESP partition device by walking up to the root disk(s)
             let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?;
             let esp_dev = root_dev.find_first_colocated_esp()?;
 
             (
-                sysroot,
                 esp_dev.path(),
                 bootloader,
                 booted_cfs.cmdline.allow_missing_fsverity,
@@ -1866,7 +1881,7 @@ pub(crate) fn setup_composefs_uki_boot(
 
     match bootloader.kind()? {
         BootloaderKind::GRUBClassic => {
-            write_grub_uki_menuentry(root_path, &setup_type, boot_label, &deploy_id, &esp_device)?
+            write_grub_uki_menuentry(&setup_type, boot_label, &deploy_id, &esp_device)?
         }
 
         BootloaderKind::BLSCompatible => write_systemd_uki_config(
@@ -2134,7 +2149,7 @@ pub(crate) async fn setup_composefs_boot(
         // which is otherwise mounted at the composefs root's own /boot) and
         // an empty `boot/efi` directory for its EFI component to discover
         // and mount the real ESP into, exactly as it would on ostree.
-        let bind_boot_path = root_setup.physical_root_path.join("boot");
+        let bind_boot_path = root_setup.physical_root_path.join(BOOT);
         crate::bootloader::install_via_bootupd(
             &root_setup.device_info,
             &root_setup.physical_root_path,
@@ -2275,6 +2290,22 @@ mod tests {
     use super::*;
     use composefs::erofs::format::FormatVersion;
 
+    #[test]
+    fn test_grub_bls_abs_entries_path() -> Result<()> {
+        let td = tempfile::tempdir()?;
+        let td = Utf8Path::from_path(td.path()).unwrap();
+        let boot = td.join(BOOT);
+        std::fs::create_dir(&boot)?;
+        // boot on the root filesystem
+        assert_eq!(grub_bls_abs_entries_path(&boot)?, "/boot");
+        // A separate filesystem
+        assert_eq!(grub_bls_abs_entries_path(Utf8Path::new("/proc"))?, "/");
+        // Nothing to check
+        assert!(grub_bls_abs_entries_path(Utf8Path::new("/")).is_err());
+        assert!(grub_bls_abs_entries_path(&td.join("nonexistent/boot")).is_err());
+        Ok(())
+    }
+
     #[test]
     fn test_replace_composefs_karg() {
         let mut cmdline =

crates/lib/src/bootc_composefs/gc.rs

@@ -536,7 +536,7 @@ pub(crate) async fn composefs_gc(
         let boot_dump = composefs_ctl::dump_files(
             &booted_cfs.repo,
             &non_bootable_img.id.to_hex(),
-            &vec![std::path::PathBuf::from("/boot")],
+            &vec![std::path::Path::new("/").join(crate::install::BOOT)],
             false,
         )
         .context("Getting dump for /boot")?;

crates/lib/src/bootloader.rs

@@ -46,7 +46,7 @@ const BOOTCTL_RANDOM_SEED_MIN_VERSION: u32 = 257;
 /// in place (bootupd will overwrite them during installation).
 // TODO: clean all ESPs on multi-device setups
 pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> {
-    let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR);
+    let efi_path = Utf8Path::new(crate::install::BOOT).join(crate::bootloader::EFI_DIR);
     let Some(esp_fd) = root
         .open_dir_optional(&efi_path)
         .context("Opening /boot/efi")?

crates/lib/src/install.rs

@@ -966,8 +966,8 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
     }
 
     // And also label /boot AKA xbootldr, if it exists
-    if rootfs_dir.try_exists("boot")? {
-        crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?;
+    if rootfs_dir.try_exists(BOOT)? {
+        crate::lsm::ensure_dir_labeled(rootfs_dir, BOOT, None, 0o755.into(), sepolicy)?;
     }
 
     // Build the list of ostree repo config options: defaults + install config
@@ -1264,7 +1264,7 @@ async fn install_container(
     if let Some(policy) = sepolicy {
         let deployment_root_meta = root.dir_metadata()?;
         let deployment_root_devino = (deployment_root_meta.dev(), deployment_root_meta.ino());
-        for d in ["ostree", "boot"] {
+        for d in ["ostree", BOOT] {
             let mut pathbuf = Utf8PathBuf::from(d);
             crate::lsm::ensure_dir_labeled_recurse(
                 &root_setup.physical_root,
@@ -1972,7 +1972,7 @@ async fn install_with_sysroot(
                     .clone()
                     .unwrap_or(rootfs.physical_root_path.clone());
                 let chroot_target = root_path.join(deployment_path.as_str());
-                let bind_boot_path = root_path.join("boot");
+                let bind_boot_path = root_path.join(BOOT);
                 crate::bootloader::install_via_bootupd(
                     &rootfs.device_info,
                     &root_path,
@@ -2226,7 +2226,7 @@ async fn install_to_filesystem_impl(
 
     // Finalize mounted filesystems
     if !rootfs.skip_finalize {
-        let bootfs = rootfs.boot.as_ref().map(|_| ("boot", "boot"));
+        let bootfs = rootfs.boot.as_ref().map(|_| ("boot", BOOT));
         for (fsname, fs) in std::iter::once(("root", ".")).chain(bootfs) {
             finalize_filesystem(fsname, &rootfs.physical_root, fs)?;
         }
@@ -2785,7 +2785,7 @@ pub(crate) async fn install_to_filesystem(
         if spec.is_empty() {
             None
         } else {
-            Some(MountSpec::new(&spec, "/boot"))
+            Some(MountSpec::new(&spec, &format!("/{BOOT}")))
         }
     } else {
         // Read /etc/fstab to get boot entry, but only use it if it's UUID-based
@@ -2795,7 +2795,7 @@ pub(crate) async fn install_to_filesystem(
             .or_else(|| {
                 boot_uuid
                     .as_deref()
-                    .map(|boot_uuid| MountSpec::new_uuid_src(boot_uuid, "/boot"))
+                    .map(|boot_uuid| MountSpec::new_uuid_src(boot_uuid, &format!("/{BOOT}")))
             })
     };
     // Ensure that we mount /boot readonly because it's really owned by bootc/ostree
@@ -2921,7 +2921,7 @@ fn read_boot_fstab_entry(root: &Dir) -> Result<Option<MountSpec>> {
         let spec = MountSpec::from_str(line)?;
 
         // Check if this is a /boot entry
-        if spec.target == "/boot" {
+        if spec.target == format!("/{BOOT}") {
             return Ok(Some(spec));
         }
     }

crates/lib/src/install/baseline.rs

@@ -25,6 +25,7 @@ use clap::ValueEnum;
 use fn_error_context::context;
 use serde::{Deserialize, Serialize};
 
+use super::BOOT;
 use super::MountSpec;
 use super::RUN_BOOTC;
 use super::RW_KARG;
@@ -879,7 +880,7 @@ pub(crate) fn install_create_rootfs(
     let bootarg = bootsrc.as_deref().map(|bootsrc| format!("boot={bootsrc}"));
     let boot = bootsrc.map(|bootsrc| MountSpec {
         source: bootsrc,
-        target: "/boot".into(),
+        target: format!("/{BOOT}"),
         fstype: MountSpec::AUTO.into(),
         options: Some("ro".into()),
     });
@@ -919,14 +920,14 @@ pub(crate) fn install_create_rootfs(
     let target_rootfs = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?;
     crate::lsm::ensure_dir_labeled(&target_rootfs, "", Some("/".into()), 0o755.into(), sepolicy)?;
     let physical_root = Dir::open_ambient_dir(&physical_root_path, cap_std::ambient_authority())?;
-    let bootfs = physical_root_path.join("boot");
+    let bootfs = physical_root_path.join(BOOT);
     // Create the underlying mount point directory, which should be labeled
-    crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;
+    crate::lsm::ensure_dir_labeled(&target_rootfs, BOOT, None, 0o755.into(), sepolicy)?;
     if let Some(bootdev) = bootdev {
         bootc_mount::mount_typed(&bootdev.path(), fstype, &bootfs)?;
     }
     // And we want to label the root mount of /boot
-    crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;
+    crate::lsm::ensure_dir_labeled(&target_rootfs, BOOT, None, 0o755.into(), sepolicy)?;
 
     // Create the EFI system partition, if applicable
     if let Some(esp_partno) = layout.esp_partno {

crates/lib/src/lints.rs

@@ -765,7 +765,7 @@ Any content here in the container image will be masked at runtime.
     check_boot,
 );
 fn check_boot(root: &Dir, config: &LintExecutionConfig) -> LintResult {
-    let Some(d) = root.open_dir_optional("boot")? else {
+    let Some(d) = root.open_dir_optional(crate::install::BOOT)? else {
         return lint_err("Missing /boot directory");
     };