UKI print dump diff

contrib/packaging/finalize-uki

@@ -31,6 +31,10 @@ mkdir -p /boot/EFI/Linux
 target=/boot/EFI/Linux/${kver}.efi
 cp "${uki_src}/${kver}.efi" "${target}"
 
+if [[ -f "${uki_src}/${kver}.dump" ]]; then
+    cp "${uki_src}/${kver}.dump" /boot
+fi
+
 # NOTE: We used to create a symlink from /usr/lib/modules/${kver}/${kver}.efi to the UKI
 # for tooling compatibility. However, composefs-boot's find_uki_components() doesn't
 # handle symlinks correctly and fails with "is not a regular file". The UKI is already

contrib/packaging/seal-uki

@@ -3,6 +3,7 @@
 set -xeuo pipefail
 
 missing_verity=()
+dumpfile_args=()
 
 while [ ! -z "${1:-}" ]; do
     case "$1" in
@@ -38,6 +39,12 @@ while [ ! -z "${1:-}" ]; do
             shift
         ;;
 
+        "--write-dumpfile-to")
+            dumpfile_args=(--write-dumpfile-to "$2")
+            shift
+            shift
+        ;;
+
         # Path to the directory containing kernel and initramfs
         "--kernel-dir")
             kernel_dir="$2"
@@ -85,4 +92,4 @@ containerukifyargs=(--rootfs "${target}")
 
 # Build the UKI using bootc container ukify
 # This computes the composefs digest, reads kargs from kargs.d, and invokes ukify
-bootc container ukify "${containerukifyargs[@]}" "${kernel_params[@]}" "${missing_verity[@]}" -- "${ukifyargs[@]}"
+bootc container ukify "${containerukifyargs[@]}" "${kernel_params[@]}" "${missing_verity[@]}" "${dumpfile_args[@]}" -- "${ukifyargs[@]}"

crates/lib/src/bootc_composefs/boot.rs

@@ -63,7 +63,7 @@
 
 use std::fs::create_dir_all;
 use std::io::{Read, Seek, SeekFrom, Write};
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 use anyhow::{Context, Result, anyhow, bail};
@@ -89,6 +89,7 @@ use composefs_ctl::composefs_boot;
 use composefs_ctl::composefs_oci;
 use fn_error_context::context;
 use linux_kernel_cmdline::utf8::{Cmdline, Parameter};
+use ostree_ext::composefs::dumpfile;
 use rustix::{mount::MountFlags, path::Arg};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
@@ -134,6 +135,14 @@ const AUTH_EXT: &str = "auth";
 /// This is relative to the ESP
 pub(crate) const BOOTC_UKI_DIR: &str = "EFI/Linux/bootc";
 
+#[derive(thiserror::Error, Debug)]
+#[error("The UKI has the wrong composefs= parameter (is '{actual}', should be '{expected}')")]
+pub(crate) struct UKIDigestMismatch {
+    pub actual: String,
+    pub expected: String,
+    pub uki_name: Option<String>,
+}
+
 pub(crate) enum BootSetupType<'a> {
     /// For initial setup, i.e. install to-disk
     Setup((&'a RootSetup, &'a State, &'a PostFetchState)),
@@ -491,7 +500,7 @@ struct BLSEntryPath {
 #[context("Setting up BLS boot")]
 pub(crate) fn setup_composefs_bls_boot(
     setup_type: BootSetupType,
-    repo: crate::store::ComposefsRepository,
+    repo: &crate::store::ComposefsRepository,
     id: &Sha512HashValue,
     entry: &ComposefsBootEntry<Sha512HashValue>,
     mounted_erofs: &Dir,
@@ -845,10 +854,15 @@ fn write_pe_to_esp(
             _ => { /* no-op */ }
         }
 
+        let file_name = file_path.file_name();
+
         if *composefs_cmdline != *uki_id {
-            anyhow::bail!(
-                "The UKI has the wrong composefs= parameter (is '{composefs_cmdline:?}', should be {uki_id:?})"
-            );
+            return Err(UKIDigestMismatch {
+                actual: composefs_cmdline.to_hex(),
+                expected: uki_id.to_hex(),
+                uki_name: file_name.map(|x| x.to_string()),
+            }
+            .into());
         }
 
         uki_reader.seek(SeekFrom::Start(0))?;
@@ -1083,7 +1097,7 @@ fn write_systemd_uki_config(
 #[context("Setting up UKI boot")]
 pub(crate) fn setup_composefs_uki_boot(
     setup_type: BootSetupType,
-    repo: crate::store::ComposefsRepository,
+    repo: &crate::store::ComposefsRepository,
     id: &Sha512HashValue,
     entries: Vec<ComposefsBootEntry<Sha512HashValue>>,
 ) -> Result<String> {
@@ -1450,7 +1464,6 @@ pub(crate) async fn setup_composefs_boot(
 
     let boot_type = BootType::from(entry);
 
-    // Unwrap Arc to pass owned repo to boot setup functions.
     let repo = Arc::try_unwrap(repo).map_err(|_| {
         anyhow::anyhow!(
             "BUG: Arc<Repository> still has other references after boot image generation"
@@ -1460,17 +1473,87 @@ pub(crate) async fn setup_composefs_boot(
     let boot_digest = match boot_type {
         BootType::Bls => setup_composefs_bls_boot(
             BootSetupType::Setup((&root_setup, &state, &postfetch)),
-            repo,
+            &repo,
             &id,
             entry,
             mounted_root.dir(),
         )?,
-        BootType::Uki => setup_composefs_uki_boot(
-            BootSetupType::Setup((&root_setup, &state, &postfetch)),
-            repo,
-            &id,
-            entries,
-        )?,
+        BootType::Uki => {
+            let uki_setup_result = setup_composefs_uki_boot(
+                BootSetupType::Setup((&root_setup, &state, &postfetch)),
+                &repo,
+                &id,
+                entries,
+            );
+
+            match uki_setup_result {
+                Ok(boot_digest) => boot_digest,
+                Err(e) => match e.downcast::<UKIDigestMismatch>() {
+                    Ok(mismatch) => {
+                        // We expect the dumpfile to be named the same as the UKI
+                        // Ex. UKI      - 6.19.14-108.fc42.x86_64.efi
+                        //     Dumpfile - 6.19.14-108.fc42.x86_64.dump
+                        let dumpfile_name = mismatch
+                            .uki_name
+                            .as_ref()
+                            .and_then(|x| x.strip_suffix(EFI_EXT).map(|x| format!("{x}.dump")));
+
+                        let Some(dumpfile_name) = &dumpfile_name else {
+                            return Err(mismatch.into());
+                        };
+
+                        let dump = composefs_ctl::dump_files(
+                            &repo,
+                            &id.to_hex(),
+                            &vec![PathBuf::from(dumpfile_name)],
+                            true,
+                        );
+
+                        let Ok(dump) = dump else {
+                            tracing::debug!("Dumpfile not found for diff");
+                            return Err(mismatch.into());
+                        };
+
+                        // SAFETY: This output is always UTF-8 compatible as it's of the form
+                        // <file-name> <object-path>
+                        let text = std::str::from_utf8(&dump)?;
+                        let obj_path = text.split_whitespace().nth(1);
+
+                        let Some(obj_path) = obj_path else {
+                            return Err(mismatch.into());
+                        };
+
+                        let tempdir = tempfile::tempdir()?;
+                        let path = tempdir.path();
+                        let tempdir = Dir::open_ambient_dir(path, ambient_authority())?;
+
+                        let mut tmpfile = tempdir.create("current")?;
+                        dumpfile::write_dumpfile(&mut tmpfile, &fs).context("Writing dumpfile")?;
+
+                        let mut cmd = std::process::Command::new("diff");
+                        let out = cmd
+                            .arg("--color=auto")
+                            .arg(
+                                root_setup
+                                    .physical_root_path
+                                    .join("sysroot/composefs/objects")
+                                    .join(obj_path),
+                            )
+                            .arg(format!("{}/current", path.display()))
+                            .status();
+
+                        // Intentionally not short-circuiting here as the real error is digest
+                        // mismtach
+                        if let Err(e) = out {
+                            tracing::warn!("diffing dumpfiles failed with Err: {e:?}");
+                        };
+
+                        return Err(mismatch.into());
+                    }
+                    Err(e) => Err(e)?,
+                },
+            }
+        }
     };
 
     write_composefs_state(

crates/lib/src/bootc_composefs/update.rs

@@ -314,15 +314,15 @@ pub(crate) async fn do_upgrade(
     let boot_digest = match boot_type {
         BootType::Bls => setup_composefs_bls_boot(
             BootSetupType::Upgrade((storage, booted_cfs, &host)),
-            repo,
+            &repo,
             &id,
             entry,
             &mounted_fs,
         )?,
 
         BootType::Uki => setup_composefs_uki_boot(
             BootSetupType::Upgrade((storage, booted_cfs, &host)),
-            repo,
+            &repo,
             &id,
             entries,
         )?,

tmt/plans/integration.fmf

@@ -292,4 +292,11 @@ execute:
     how: fmf
     test:
       - /tmt/tests/tests/test-46-etc-merge-conflict
+
+/plan-48-composefs-uki-dumpfile:
+  summary: Test composefs garbage collection for UKI
+  discover:
+    how: fmf
+    test:
+      - /tmt/tests/tests/test-48-composefs-uki-dumpfile
 # END GENERATED PLANS

tmt/tests/booted/tap.nu

@@ -111,13 +111,18 @@ export def make_uki_containerfile [containerfile: string] {
         FROM base as sealed-uki
         RUN --network=none --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp \\
             --mount=type=bind,from=base-final,src=/,target=/run/target \\
-            --mount=type=bind,from=kernel,src=/,target=/run/kernel \\
+            --mount=type=bind,from=kernel,src=/,target=/run/kernel <<-EOF
+
+              kver=$\(bootc container inspect --rootfs /run/kernel --json | jq -r '.kernel.version'\)
+
               /usr/bin/seal-uki \\
                   --target /run/target \\
                   --output /out \\
                   --secrets /run/secrets ($allow_missing_verity) \\
-                  --kernel-dir /run/kernel/boot/$\(bootc container inspect --rootfs /run/kernel --json | jq -r '.kernel.version'\) \\
+                  --kernel-dir /run/kernel/boot/${kver} \\
+                  --write-dumpfile-to /out/${kver}.dump \\
                   --seal-state ($seal_state)
+        EOF
 
         FROM base-final
 

tmt/tests/booted/test-composefs-uki-dumpfile.nu

@@ -0,0 +1,60 @@
+# number: 48
+# tmt:
+#   summary: Test composefs garbage collection for UKI
+#   duration: 30m
+
+use std assert
+use tap.nu
+
+if not (tap is_composefs) {
+    exit 0
+}
+
+# bootc status
+let st = bootc status --json | from json
+let booted = $st.status.booted.image
+
+let is_uki = (($st.status.booted.composefs.bootType | str downcase) == "uki")
+
+if not $is_uki {
+    exit 0
+}
+
+def first_boot [] {
+    bootc image copy-to-storage
+
+    mut containerfile = $"
+        FROM localhost/bootc as base
+        RUN touch /usr/share/accepted-file
+    "
+
+    $containerfile = (tap make_uki_containerfile $containerfile)
+
+    $containerfile += "
+        RUN touch /usr/share/new-file
+    "
+
+    echo $containerfile | podman build -t localhost/dump-diff . -f -
+
+    let result = do { bootc switch --transport containers-storage localhost/dump-diff } | complete
+
+    let actual_digest = ./bootc internals cfs oci compute-id $"@(podman images --no-trunc | grep dump-diff | awk '{print $3}')"
+
+    assert ($result.exit_code != 0) "bootc switch should fail"
+
+    print ($result.stderr)
+
+    assert ($result.stderr | str contains "The UKI has the wrong composefs= parameter") $"Expected 'The UKI has the wrong composefs= parameter' in stderr"
+    assert ($result.stderr | str contains $"should be '($actual_digest)'") $"Expected digest to be ($actual_digest) in stderr"
+    assert ($result.stderr | str contains "/usr/share/new-file") $"Expected '/usr/share/new-file' in stderr"
+
+    tap ok
+}
+
+def main [] {
+    match $env.TMT_REBOOT_COUNT? {
+        null | "0" => first_boot,
+        $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } },
+    }
+}
+

tmt/tests/tests.fmf

@@ -183,3 +183,8 @@ check:
   summary: Verify etc merge conflicts are caught during upgrade, not finalization
   duration: 15m
   test: nu booted/test-etc-merge-conflict.nu
+
+/test-48-composefs-uki-dumpfile:
+  summary: Test composefs garbage collection for UKI
+  duration: 30m
+  test: nu booted/test-composefs-uki-dumpfile.nu