From c64357be4900081ec678f7066e0d903c32a295f4 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Wed, 26 Aug 2026 18:35:43 +0200 Subject: [PATCH] efi: error when the requested bootloader has no component get_efi_component_from_usr filters out components belonging to *other* bootloaders, but components belonging to no bootloader are never removed -- notably shim, which appears in no bootloader's to_remove set. A non-empty result therefore does not mean the requested bootloader was found. So with --bootloader systemd against an image whose systemd-boot payload is absent, or laid out somewhere other than //EFI, discovery returns just [shim], install copies shim to the ESP, and the command reports success. The machine is left with a first stage and nothing behind it, and nothing in the output says so. That is not hypothetical. Fedora's systemd-boot package installs its binary at usr/lib/efi/systemd-boot//grubx64.efi, one level above the EFI directory the walker matches on, so it is invisible today: https://src.fedoraproject.org/rpms/systemd-boot/pull-request/4 Check explicitly that a component providing the requested bootloader survived the filter, and name the components that were found when it did not, so that a package shipping its payload at the wrong path is distinguishable from one shipping no payload at all. Discovering nothing at all is now also an error for a filtered request, where before it returned None. Only unfiltered discovery keeps that: None is how the caller learns the new layout is not in use and falls back to the legacy update directory, and that directory is copied wholesale with no way to honour a bootloader selection -- so reaching it with a request for one silently installs whatever it happens to contain. Assisted-by: AI --- src/efi.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/src/efi.rs b/src/efi.rs index 21750435..f13118a0 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -930,6 +930,9 @@ pub struct EFIComponent { /// Get EFIComponents from e.g. usr/lib/efi, like "usr/lib/efi///EFI" /// Filter the components by Bootloader, if Bootloader is None, no filtering is performed +/// +/// Errors if a Bootloader is requested but no component provides it, so that +/// a missing payload fails here rather than silently producing a half-installed ESP. fn get_efi_component_from_usr<'a>( sysroot: &'a Utf8Path, usr_path: &'a str, @@ -965,13 +968,15 @@ fn get_efi_component_from_usr<'a>( }) .collect(); - if components.len() == 0 { - return Ok(None); - } components.sort_by(|a, b| a.name.cmp(&b.name)); let Some(bootloader) = bootloader else { - return Ok(Some(components)); + // Unfiltered discovery: an empty result is how callers learn that the + // new layout is not in use, so that they can fall back to the legacy + // update directory. A filtered request cannot fall back that way -- + // the legacy directory is copied wholesale, with no way to honour a + // bootloader selection -- so it goes on to the check below instead. + return Ok((!components.is_empty()).then_some(components)); }; // Remove all EFI Components not associated with the bootloader @@ -980,11 +985,32 @@ fn get_efi_component_from_usr<'a>( .map(|b| b.efi_component_name()) .collect::>(); + let found = if components.is_empty() { + "none".to_string() + } else { + components + .iter() + .map(|comp| comp.name.as_str()) + .collect::>() + .join(", ") + }; + let efi_comps = components .into_iter() .filter(|comp| !to_remove.contains(&comp.name.as_str())) .collect::>(); + // Components that belong to no bootloader such as shim are never + // filtered out, so a non-empty efi_comps therefore does not mean + // the requested bootloader was found. Check explicitly. + let wanted = bootloader.efi_component_name(); + if !efi_comps.iter().any(|comp| comp.name == wanted) { + anyhow::bail!( + "Bootloader '{bootloader}' was requested, but no '{wanted}' component was found \ + in {usr_path}; expected {usr_path}/{wanted}//EFI (found: {found})" + ); + } + Ok(Some(efi_comps)) } @@ -1236,11 +1262,58 @@ Boot0003* test"; ]) ); - // Test with empty directory - should return None + // Test with filtering for Systemd while no systemd-boot component exists - should + // error rather than quietly returning shim on its own, which would install a first + // stage with no second stage behind it. + let err = get_efi_component_from_usr(utf8_tpath, EFILIB, Some(Bootloader::Systemd)) + .expect_err("missing systemd-boot component should be an error"); + let err = err.to_string(); + assert!( + err.contains("no 'systemd-boot' component") + && err.contains("found: grub-cc, grub2, shim"), + "unexpected error: {err}" + ); + + // systemd-boot structure. Note the binary is named grubx64.efi: that is the second + // stage filename baked into shim. + std::fs::create_dir_all(efi_path.join("systemd-boot/261.2-4.fc45/EFI/fedora"))?; + std::fs::File::create(efi_path.join("systemd-boot/261.2-4.fc45/EFI/fedora/grubx64.efi"))?; + + // Test with filtering for Systemd - should only return shim and systemd-boot + let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, Some(Bootloader::Systemd))?; + assert_eq!( + efi_comps, + Some(vec![ + EFIComponent { + name: "shim".to_string(), + version: "16.1-5".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/shim/16.1-5/EFI"), + }, + EFIComponent { + name: "systemd-boot".to_string(), + version: "261.2-4.fc45".to_string(), + path: Utf8PathBuf::from("usr/lib/efi/systemd-boot/261.2-4.fc45/EFI"), + }, + ]) + ); + + // Test with empty directory - unfiltered discovery returns None, which is + // how the caller learns to fall back to the legacy update directory. std::fs::remove_dir_all(&efi_path)?; std::fs::create_dir_all(&efi_path)?; let efi_comps = get_efi_component_from_usr(utf8_tpath, EFILIB, None)?; assert_eq!(efi_comps, None); + + // ...but a request for a specific bootloader still errors, rather than + // falling back to a directory that is copied wholesale and so cannot + // honour the request. + let err = get_efi_component_from_usr(utf8_tpath, EFILIB, Some(Bootloader::Systemd)) + .expect_err("missing systemd-boot component should be an error"); + let err = err.to_string(); + assert!( + err.contains("no 'systemd-boot' component") && err.contains("found: none"), + "unexpected error: {err}" + ); Ok(()) }