From 22780e610261748982f0f657e12387804ced25bb Mon Sep 17 00:00:00 2001 From: Alex Zenla Date: Thu, 6 Nov 2025 11:52:00 -0500 Subject: [PATCH] chore(eficore): decouple the shim support from the image load callsites --- crates/boot/src/actions/chainload.rs | 17 ++-- crates/boot/src/drivers.rs | 12 ++- crates/eficore/src/lib.rs | 3 + crates/eficore/src/loader.rs | 141 +++++++++++++++++++++++++++ crates/eficore/src/loader/source.rs | 25 +++++ crates/eficore/src/shim.rs | 73 +------------- 6 files changed, 189 insertions(+), 82 deletions(-) create mode 100644 crates/eficore/src/loader.rs create mode 100644 crates/eficore/src/loader/source.rs diff --git a/crates/boot/src/actions/chainload.rs b/crates/boot/src/actions/chainload.rs index 6c1176d..fdebfc0 100644 --- a/crates/boot/src/actions/chainload.rs +++ b/crates/boot/src/actions/chainload.rs @@ -5,9 +5,10 @@ use alloc::rc::Rc; use anyhow::{Context, Result, bail}; use edera_sprout_config::actions::chainload::ChainloadConfiguration; use eficore::bootloader_interface::BootloaderInterface; +use eficore::loader::source::ImageSource; +use eficore::loader::{ImageLoadRequest, ImageLoader}; use eficore::media_loader::MediaLoaderHandle; use eficore::media_loader::constants::linux::LINUX_EFI_INITRD_MEDIA_GUID; -use eficore::shim::{ShimInput, ShimSupport}; use log::error; use uefi::CString16; use uefi::proto::loaded_image::LoadedImage; @@ -24,13 +25,17 @@ pub fn chainload(context: Rc, configuration: &ChainloadConfigurat ) .context("unable to resolve chainload path")?; - // Load the image to chainload using the shim support integration. + // Create a new image load request with the current image and the resolved path. + let request = ImageLoadRequest::new(sprout_image, ImageSource::ResolvedPath(&resolved)); + + // Load the image to chainload using the image loader support module. // It will determine if the image needs to be loaded via the shim or can be loaded directly. - let image = ShimSupport::load(sprout_image, ShimInput::ResolvedPath(&resolved))?; + let image = ImageLoader::load(request)?; // Open the LoadedImage protocol of the image to chainload. - let mut loaded_image_protocol = uefi::boot::open_protocol_exclusive::(image) - .context("unable to open loaded image protocol")?; + let mut loaded_image_protocol = + uefi::boot::open_protocol_exclusive::(*image.handle()) + .context("unable to open loaded image protocol")?; // Stamp and combine the options to pass to the image. let options = @@ -87,7 +92,7 @@ pub fn chainload(context: Rc, configuration: &ChainloadConfigurat // This call might return, or it may pass full control to another image that will never return. // Capture the result to ensure we can return an error if the image fails to start, but only // after the optional initrd has been unregistered. - let result = uefi::boot::start_image(image); + let result = uefi::boot::start_image(*image.handle()); // Unregister the initrd if it was registered. if let Some(initrd_handle) = initrd_handle diff --git a/crates/boot/src/drivers.rs b/crates/boot/src/drivers.rs index accc005..1f95abc 100644 --- a/crates/boot/src/drivers.rs +++ b/crates/boot/src/drivers.rs @@ -5,7 +5,8 @@ use alloc::rc::Rc; use alloc::string::String; use anyhow::{Context, Result}; use edera_sprout_config::drivers::DriverDeclaration; -use eficore::shim::{ShimInput, ShimSupport}; +use eficore::loader::source::ImageSource; +use eficore::loader::{ImageLoadRequest, ImageLoader}; use log::info; use uefi::boot::SearchType; @@ -21,14 +22,17 @@ fn load_driver(context: Rc, driver: &DriverDeclaration) -> Result ) .context("unable to resolve path to driver")?; - // Load the driver image using the shim support integration. + // Create an image load request with the current image and the resolved path. + let request = ImageLoadRequest::new(sprout_image, ImageSource::ResolvedPath(&resolved)); + + // Load the driver image using the image loader support module. // It will determine if the image needs to be loaded via the shim or can be loaded directly. - let image = ShimSupport::load(sprout_image, ShimInput::ResolvedPath(&resolved))?; + let image = ImageLoader::load(request)?; // Start the driver image, this is expected to return control to sprout. // There is no guarantee that the driver will actually return control as it is // just a standard EFI image. - uefi::boot::start_image(image).context("unable to start driver image")?; + uefi::boot::start_image(*image.handle()).context("unable to start driver image")?; Ok(()) } diff --git a/crates/eficore/src/lib.rs b/crates/eficore/src/lib.rs index 0488c78..7449d82 100644 --- a/crates/eficore/src/lib.rs +++ b/crates/eficore/src/lib.rs @@ -6,6 +6,9 @@ extern crate alloc; /// EFI handle helpers. pub mod handle; +/// Load and start EFI images. +pub mod loader; + /// Logging support for EFI applications. pub mod logger; diff --git a/crates/eficore/src/loader.rs b/crates/eficore/src/loader.rs new file mode 100644 index 0000000..7958560 --- /dev/null +++ b/crates/eficore/src/loader.rs @@ -0,0 +1,141 @@ +use crate::loader::source::ImageSource; +use crate::secure::SecureBoot; +use crate::shim::hook::SecurityHook; +use crate::shim::{ShimInput, ShimSupport}; +use anyhow::{Context, Result, bail}; +use log::warn; +use uefi::Handle; +use uefi::boot::LoadImageSource; + +/// Represents EFI image sources generically. +pub mod source; + +/// Handle to a loaded EFI image. +pub struct ImageHandle { + /// Handle to the loaded image. + handle: Handle, +} + +impl ImageHandle { + /// Create a new image handle based on a handle from the UEFI stack. + pub fn new(handle: Handle) -> Self { + Self { handle } + } + + /// Retrieve the underlying handle. + pub fn handle(&self) -> &Handle { + &self.handle + } +} + +/// Request to load an image from a source, with support for additional validation features. +pub struct ImageLoadRequest<'source> { + /// Handle to the current image. + current_image: Handle, + /// Source of the image to load. + source: ImageSource<'source>, +} + +impl<'source> ImageLoadRequest<'source> { + /// Create a new image load request with a current image and a source. + pub fn new(current_image: Handle, source: ImageSource<'source>) -> Self { + Self { + current_image, + source, + } + } + + /// Retrieve the current image. + pub fn current_image(&self) -> &Handle { + &self.current_image + } + + /// Retrieve the source of the image to load. + pub fn source(&'source self) -> &'source ImageSource<'source> { + &self.source + } + + /// Convert the request into a source. + pub fn into_source(self) -> ImageSource<'source> { + self.source + } +} + +/// EFI image loader. +pub struct ImageLoader; + +impl ImageLoader { + /// Load an image using the image `request` which allows + pub fn load(request: ImageLoadRequest) -> Result { + // Determine whether Secure Boot is enabled. + let secure_boot = + SecureBoot::enabled().context("unable to determine if secure boot is enabled")?; + + // Determine whether the shim is loaded. + let shim_loaded = ShimSupport::loaded().context("unable to determine if shim is loaded")?; + + // Determine whether the shim loader is available. + let shim_loader_available = ShimSupport::loader_available() + .context("unable to determine if shim loader is available")?; + + // Determines whether LoadImage in Boot Services must be patched. + // Version 16 of the shim doesn't require extra effort to load Secure Boot binaries. + // If the image loader is installed, we can skip over the security hook. + let requires_security_hook = secure_boot && shim_loaded && !shim_loader_available; + + // If the security hook is required, we will bail for now. + if requires_security_hook { + // Install the security hook, if possible. If it's not, this is necessary to continue, + // so we should bail. + let installed = SecurityHook::install().context("unable to install security hook")?; + if !installed { + bail!("unable to install security hook required for this platform"); + } + } + + // If the shim is loaded, we will need to retain the shim protocol to allow + // loading multiple images. + if shim_loaded { + // Retain the shim protocol after loading the image. + ShimSupport::retain()?; + } + + // Clone the current image handle to use for loading the image. + let current_image = *request.current_image(); + + // Converts the source to a shim input with an owned data buffer. + let input = ShimInput::from(request.into_source()) + .into_owned_data_buffer() + .context("unable to convert input to loaded data buffer")?; + + // Constructs a LoadImageSource from the input. + let source = LoadImageSource::FromBuffer { + buffer: input.buffer().context("unable to get buffer from input")?, + file_path: input.file_path(), + }; + + // Loads the image using Boot Services LoadImage function. + let result = uefi::boot::load_image(current_image, source).context("unable to load image"); + + // If the security override is required, we will uninstall the security hook. + if requires_security_hook { + let uninstall_result = crate::shim::hook::SecurityHook::uninstall(); + // Ensure we don't mask load image errors if uninstalling fails. + if result.is_err() + && let Err(uninstall_error) = &uninstall_result + { + // Warn on the error since the load image error is more important. + warn!("unable to uninstall security hook: {}", uninstall_error); + } else { + // Otherwise, ensure we handle the original uninstallation result. + uninstall_result?; + } + } + + // Assert the result and grab the handle. + let handle = result?; + + // Retrieve the handle from the result and make a new image handle. + Ok(ImageHandle::new(handle)) + } +} diff --git a/crates/eficore/src/loader/source.rs b/crates/eficore/src/loader/source.rs new file mode 100644 index 0000000..1e01613 --- /dev/null +++ b/crates/eficore/src/loader/source.rs @@ -0,0 +1,25 @@ +use crate::path::ResolvedPath; +use crate::shim::ShimInput; + +/// Represents a source of an EFI image. +pub enum ImageSource<'source> { + /// The image is located at the specified path that has been resolved. + ResolvedPath(&'source ResolvedPath), + /// The image is located in a buffer. + DataBuffer { + /// Optional path to the image. + path: Option<&'source ResolvedPath>, + /// Buffer containing the image. + buffer: &'source [u8], + }, +} + +/// Implement conversion from `ImageSource` to `ShimInput`, which is used by the shim support code. +impl<'source> From> for ShimInput<'source> { + fn from(value: ImageSource<'source>) -> Self { + match value { + ImageSource::ResolvedPath(path) => ShimInput::ResolvedPath(path), + ImageSource::DataBuffer { path, buffer } => ShimInput::DataBuffer(path, buffer), + } + } +} diff --git a/crates/eficore/src/shim.rs b/crates/eficore/src/shim.rs index 56777d2..bd8e1c5 100644 --- a/crates/eficore/src/shim.rs +++ b/crates/eficore/src/shim.rs @@ -1,6 +1,4 @@ use crate::path::ResolvedPath; -use crate::secure::SecureBoot; -use crate::shim::hook::SecurityHook; use crate::variables::{VariableClass, VariableController}; use alloc::boxed::Box; use alloc::string::ToString; @@ -8,9 +6,6 @@ use alloc::vec::Vec; use anyhow::{Context, Result, anyhow, bail}; use core::ffi::c_void; use core::pin::Pin; -use log::warn; -use uefi::Handle; -use uefi::boot::LoadImageSource; use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly}; use uefi::proto::device_path::{DevicePath, FfiDevicePath}; use uefi::proto::unsafe_protocol; @@ -18,7 +13,7 @@ use uefi_raw::table::runtime::VariableVendor; use uefi_raw::{Guid, Status, guid}; /// Security hook support. -mod hook; +pub mod hook; /// Support for the shim loader application for Secure Boot. pub struct ShimSupport; @@ -237,72 +232,6 @@ impl ShimSupport { .unwrap_or(ShimVerificationOutput::VerifiedDataNotLoaded)) } - /// Load the image specified by the `input` and returns an image handle. - pub fn load(current_image: Handle, input: ShimInput) -> Result { - // Determine whether Secure Boot is enabled. - let secure_boot = - SecureBoot::enabled().context("unable to determine if secure boot is enabled")?; - - // Determine whether the shim is loaded. - let shim_loaded = Self::loaded().context("unable to determine if shim is loaded")?; - - // Determine whether the shim loader is available. - let shim_loader_available = - Self::loader_available().context("unable to determine if shim loader is available")?; - - // Determines whether LoadImage in Boot Services must be patched. - // Version 16 of the shim doesn't require extra effort to load Secure Boot binaries. - // If the image loader is installed, we can skip over the security hook. - let requires_security_hook = secure_boot && shim_loaded && !shim_loader_available; - - // If the security hook is required, we will bail for now. - if requires_security_hook { - // Install the security hook, if possible. If it's not, this is necessary to continue, - // so we should bail. - let installed = SecurityHook::install().context("unable to install security hook")?; - if !installed { - bail!("unable to install security hook required for this platform"); - } - } - - // If the shim is loaded, we will need to retain the shim protocol to allow - // loading multiple images. - if shim_loaded { - // Retain the shim protocol after loading the image. - Self::retain()?; - } - - // Converts the shim input to an owned data buffer. - let input = input - .into_owned_data_buffer() - .context("unable to convert input to loaded data buffer")?; - - // Constructs a LoadImageSource from the input. - let source = LoadImageSource::FromBuffer { - buffer: input.buffer().context("unable to get buffer from input")?, - file_path: input.file_path(), - }; - - // Loads the image using Boot Services LoadImage function. - let result = uefi::boot::load_image(current_image, source).context("unable to load image"); - - // If the security override is required, we will uninstall the security hook. - if requires_security_hook { - let uninstall_result = SecurityHook::uninstall(); - // Ensure we don't mask load image errors if uninstalling fails. - if result.is_err() - && let Err(uninstall_error) = &uninstall_result - { - // Warn on the error since the load image error is more important. - warn!("unable to uninstall security hook: {}", uninstall_error); - } else { - // Otherwise, ensure we handle the original uninstallation result. - uninstall_result?; - } - } - result - } - /// Set the ShimRetainProtocol variable to indicate that shim should retain the protocols /// for the full lifetime of boot services. pub fn retain() -> Result<()> {