sprout: version 0.0.14

Cargo.lock

@@ -116,7 +116,7 @@ dependencies = [
 
 [[package]]
 name = "edera-sprout"
-version = "0.0.13"
+version = "0.0.14"
 dependencies = [
  "anyhow",
  "image",

Cargo.toml

@@ -2,7 +2,7 @@
 name = "edera-sprout"
 description = "Modern UEFI bootloader"
 license = "Apache-2.0"
-version = "0.0.13"
+version = "0.0.14"
 homepage = "https://sprout.edera.dev"
 repository = "https://github.com/edera-dev/sprout"
 edition = "2024"

README.md

@@ -18,6 +18,9 @@ existing UEFI bootloader or booted by the hardware directly.
 
 Sprout is licensed under Apache 2.0 and is open to modifications and contributions.
 
+**IMPORTANT WARNING**: Sprout does not support UEFI Secure Boot yet.
+See [this issue](https://github.com/edera-dev/sprout/issues/20) for updates.
+
 ## Background
 
 At [Edera] we make compute isolation technology for a wide variety of environments, often ones we do not fully control.
@@ -55,7 +58,7 @@ The boot menu mechanism is very rudimentary.
 ### Current
 
 - [x] Loadable driver support
-- [x] [Bootloader specification (BLS)](https://uapi-group.org/specifications/specs/boot_loader_specification/) support
+- [x] Basic [Bootloader specification (BLS)](https://uapi-group.org/specifications/specs/boot_loader_specification/) support
 - [x] Chainload support
 - [x] Linux boot support via EFI stub
 - [x] Windows boot support via chainload
@@ -65,15 +68,18 @@ The boot menu mechanism is very rudimentary.
 
 ### Roadmap
 
-- [ ] Full-featured boot menu
+- [ ] [Bootloader interface support](https://github.com/edera-dev/sprout/issues/21)
-- [ ] Secure Boot support: work in progress
+- [ ] [BLS specification conformance](https://github.com/edera-dev/sprout/issues/2)
-- [ ] UKI support: partial
+- [ ] [Full-featured boot menu](https://github.com/edera-dev/sprout/issues/1)
-- [ ] multiboot2 support
+- [ ] [Secure Boot support](https://github.com/edera-dev/sprout/issues/20): work in progress
-- [ ] Linux boot protocol (boot without EFI stub)
+- [ ] [UKI support](https://github.com/edera-dev/sprout/issues/6): partial
+- [ ] [multiboot2 support](https://github.com/edera-dev/sprout/issues/7)
+- [ ] [Linux boot protocol (boot without EFI stub)](https://github.com/edera-dev/sprout/issues/7)
 
 ## Concepts
 
 - drivers: loadable EFI modules that can add functionality to the EFI system.
+- autoconfiguration: code that can automatically generate sprout.toml based on the EFI environment.
 - actions: executable code with a configuration that can be run by various other sprout concepts.
 - generators: code that can generate boot entries based on inputs or runtime code.
 - extractors: code that can extract values from the EFI environment.

src/autoconfigure/bls.rs

@@ -31,7 +31,7 @@ pub fn scan(
         .to_string(DisplayOnly(false), AllowShortcuts(false))
         .context("unable to convert device root to string")?
         .to_string();
-    // Add a trailing slash to the root to ensure the path is valid.
+    // Add a trailing forward-slash to the root to ensure the device root is completed.
     root.push('/');
 
     // Generate a unique hash of the root path.

src/autoconfigure/linux.rs

@@ -6,7 +6,6 @@ use crate::generators::GeneratorDeclaration;
 use crate::generators::list::ListConfiguration;
 use crate::utils;
 use anyhow::{Context, Result};
-use log::info;
 use std::collections::BTreeMap;
 use uefi::CString16;
 use uefi::fs::{FileSystem, Path};
@@ -89,7 +88,7 @@ fn scan_directory(filesystem: &mut FileSystem, path: &str) -> Result<Vec<KernelP
 
         // Find a matching initramfs, if any.
         let mut initramfs_prefix_iter = INITRAMFS_PREFIXES.iter();
-        let initramfs = loop {
+        let matched_initramfs_path = loop {
             let Some(prefix) = initramfs_prefix_iter.next() else {
                 break None;
             };
@@ -112,7 +111,7 @@ fn scan_directory(filesystem: &mut FileSystem, path: &str) -> Result<Vec<KernelP
         let mut kernel = path.clone();
         kernel.push(Path::new(&item.file_name()));
         let kernel = kernel.to_string();
-        let initramfs = initramfs.map(|initramfs| initramfs.to_string());
+        let initramfs = matched_initramfs_path.map(|initramfs_path| initramfs_path.to_string());
 
         // Produce a kernel pair.
         let pair = KernelPair { kernel, initramfs };
@@ -135,7 +134,7 @@ pub fn scan(
         .to_string(DisplayOnly(false), AllowShortcuts(false))
         .context("unable to convert device root to string")?
         .to_string();
-    // Add a trailing slash to the root to ensure the path is valid.
+    // Add a trailing forward-slash to the root to ensure the device root is completed.
     root.push('/');
 
     // Generate a unique hash of the root path.
@@ -215,8 +214,6 @@ pub fn scan(
         },
     );
 
-    info!("{:?}", config);
-
     // We had a Linux kernel, so return true to indicate something was found.
     Ok(true)
 }

src/autoconfigure/windows.rs

@@ -39,7 +39,7 @@ pub fn scan(
         .to_string(DisplayOnly(false), AllowShortcuts(false))
         .context("unable to convert device root to string")?
         .to_string();
-    // Add a trailing slash to the root to ensure the path is valid.
+    // Add a trailing forward-slash to the root to ensure the device root is completed.
     root.push('/');
 
     // Generate a unique hash of the root path.

src/context.rs

@@ -168,13 +168,13 @@ impl SproutContext {
         let mut current_values = self.all_values();
 
         // To ensure that there is no possible infinite loop, we need to check
-        // the number of iterations. If it exceeds 100, we bail.
+        // the number of iterations. If it exceeds CONTEXT_FINALIZE_ITERATION_LIMIT, we bail.
         let mut iterations: usize = 0;
         loop {
             iterations += 1;
 
             if iterations > CONTEXT_FINALIZE_ITERATION_LIMIT {
-                bail!("infinite loop detected in context finalization");
+                bail!("maximum number of replacement iterations reached while finalizing context");
             }
 
             let mut did_change = false;

src/extractors/filesystem_device_match.rs

@@ -10,16 +10,17 @@ use uefi::proto::device_path::DevicePath;
 use uefi::proto::media::file::{File, FileSystemVolumeLabel};
 use uefi::proto::media::fs::SimpleFileSystem;
 use uefi::proto::media::partition::PartitionInfo;
-use uefi::{CString16, Guid};
+use uefi::{CString16, Guid, Handle};
 use uefi_raw::Status;
 
 /// The filesystem device match extractor.
 /// This extractor finds a filesystem using some search criteria and returns
 /// the device root path that can concatenated with subpaths to access files
 /// on a particular filesystem.
+/// The fallback value can be used to provide a value if no match is found.
 ///
-/// This function only requires all the criteria to match.
+/// This extractor requires all the criteria to match. If no criteria is provided,
-/// The fallback value can be used to provide a value if none is found.
+/// an error is returned.
 #[derive(Serialize, Deserialize, Debug, Default, Clone)]
 pub struct FilesystemDeviceMatchExtractor {
     /// Matches a filesystem that has the specified label.
@@ -40,6 +41,48 @@ pub struct FilesystemDeviceMatchExtractor {
     pub fallback: Option<String>,
 }
 
+/// Represents the partition UUIDs for a filesystem.
+struct PartitionIds {
+    /// The UUID of the partition.
+    partition_uuid: Guid,
+    /// The type UUID of the partition.
+    type_uuid: Guid,
+}
+
+/// Fetches the partition UUIDs for the specified filesystem handle.
+fn fetch_partition_uuids(handle: Handle) -> Result<Option<PartitionIds>> {
+    // Open the partition info protocol for this handle.
+    let partition_info = uefi::boot::open_protocol_exclusive::<PartitionInfo>(handle);
+
+    match partition_info {
+        Ok(partition_info) => {
+            // GPT partitions have a unique partition GUID.
+            // MBR does not.
+            if let Some(gpt) = partition_info.gpt_partition_entry() {
+                let uuid = gpt.unique_partition_guid;
+                let type_uuid = gpt.partition_type_guid;
+                Ok(Some(PartitionIds {
+                    partition_uuid: uuid,
+                    type_uuid: type_uuid.0,
+                }))
+            } else {
+                Ok(None)
+            }
+        }
+
+        Err(error) => {
+            // If the filesystem does not have a partition, that is okay.
+            if error.status() == Status::NOT_FOUND || error.status() == Status::UNSUPPORTED {
+                Ok(None)
+            } else {
+                // We should still handle other errors gracefully.
+                Err(error).context("unable to open filesystem partition info")?;
+                unreachable!()
+            }
+        }
+    }
+}
+
 /// Extract a filesystem device path using the specified `context` and `extractor` configuration.
 pub fn extract(
     context: Rc<SproutContext>,
@@ -56,56 +99,28 @@ pub fn extract(
 
         // Extract the partition info for this filesystem.
         // There is no guarantee that the filesystem has a partition.
-        let partition_info = {
+        let partition_info =
-            // Open the partition info protocol for this handle.
+            fetch_partition_uuids(handle).context("unable to fetch partition info")?;
-            let partition_info = uefi::boot::open_protocol_exclusive::<PartitionInfo>(handle);
-
-            match partition_info {
-                Ok(partition_info) => {
-                    // GPT partitions have a unique partition GUID.
-                    // MBR does not.
-                    if let Some(gpt) = partition_info.gpt_partition_entry() {
-                        let uuid = gpt.unique_partition_guid;
-                        let type_uuid = gpt.partition_type_guid;
-                        Some((uuid, type_uuid.0))
-                    } else {
-                        None
-                    }
-                }
-
-                Err(error) => {
-                    // If the filesystem does not have a partition, that is okay.
-                    if error.status() == Status::NOT_FOUND || error.status() == Status::UNSUPPORTED
-                    {
-                        None
-                    } else {
-                        // We should still handle other errors gracefully.
-                        Err(error).context("unable to open filesystem partition info")?;
-                        unreachable!()
-                    }
-                }
-            }
-        };
 
         // Check if the partition info matches partition uuid criteria.
-        if let Some((partition_uuid, _partition_type_guid)) = partition_info
+        if let Some(ref partition_info) = partition_info
             && let Some(ref has_partition_uuid) = extractor.has_partition_uuid
         {
             let parsed_uuid = Guid::from_str(has_partition_uuid)
                 .map_err(|e| anyhow!("unable to parse has-partition-uuid: {}", e))?;
-            if partition_uuid != parsed_uuid {
+            if partition_info.partition_uuid != parsed_uuid {
                 continue;
             }
             has_match = true;
         }
 
         // Check if the partition info matches partition type uuid criteria.
-        if let Some((_partition_uuid, partition_type_guid)) = partition_info
+        if let Some(ref partition_info) = partition_info
             && let Some(ref has_partition_type_uuid) = extractor.has_partition_type_uuid
         {
             let parsed_uuid = Guid::from_str(has_partition_type_uuid)
                 .map_err(|e| anyhow!("unable to parse has-partition-type-uuid: {}", e))?;
-            if partition_type_guid != parsed_uuid {
+            if partition_info.type_uuid != parsed_uuid {
                 continue;
             }
             has_match = true;

src/generators/bls/entry.rs

@@ -41,7 +41,8 @@ impl FromStr for BlsEntry {
                 continue;
             }
 
-            // Split the line once by whitespace.
+            // Split the line once by whitespace. This technically includes newlines but since
+            // the lines iterator is used, there should never be a newline here.
             let Some((key, value)) = line.split_once(char::is_whitespace) else {
                 continue;
             };

src/menu.rs

@@ -40,8 +40,17 @@ fn read(input: &mut Input, timeout: &Duration) -> Result<MenuOperation> {
         uefi::boot::create_event_ex(EventType::TIMER, Tpl::CALLBACK, None, None, None)
             .context("unable to create timer event")?
     };
+
     // The timeout is in increments of 100 nanoseconds.
-    let trigger = TimerTrigger::Relative(timeout.as_nanos() as u64 / 100);
+    let timeout_hundred_nanos = timeout.as_nanos() / 100;
+
+    // Check if the timeout is too large to fit into an u64.
+    if timeout_hundred_nanos > u64::MAX as u128 {
+        bail!("timeout duration overflow");
+    }
+
+    // Set a timer to trigger after the specified duration.
+    let trigger = TimerTrigger::Relative(timeout_hundred_nanos as u64);
     uefi::boot::set_timer(&timer_event, trigger).context("unable to set timeout timer")?;
 
     let mut events = vec![timer_event, key_event];
@@ -50,6 +59,7 @@ fn read(input: &mut Input, timeout: &Duration) -> Result<MenuOperation> {
         .context("unable to wait for event")?;
 
     // Close the timer event that we acquired.
+    // We don't need to close the key event because it is owned globally.
     if let Some(timer_event) = events.into_iter().next() {
         uefi::boot::close_event(timer_event).context("unable to close timer event")?;
     }

src/utils/media_loader.rs

@@ -51,6 +51,11 @@ impl MediaLoaderHandle {
     /// The next call will pass a buffer of the right size, and we should copy
     /// data into that buffer, checking whether it is safe to copy based on
     /// the buffer size.
+    ///
+    /// SAFETY: `this.address` and `this.length` are set by leaking a Box<[u8]>, so we can
+    /// be sure their pointers are valid when this is called. The caller must call this function
+    /// while inside UEFI boot services to ensure pointers are valid. Copying to `buffer` is
+    /// assumed valid because the caller must ensure `buffer` is valid by function contract.
     unsafe extern "efiapi" fn load_file(
         this: *mut MediaLoaderProtocol,
         file_path: *const DevicePathProtocol,
@@ -155,7 +160,7 @@ impl MediaLoaderHandle {
 
         // Install a protocol interface for the device path.
         // This ensures it can be located by other EFI programs.
-        let mut handle = unsafe {
+        let primary_handle = unsafe {
             uefi::boot::install_protocol_interface(
                 None,
                 &DevicePathProtocol::GUID,
@@ -178,25 +183,54 @@ impl MediaLoaderHandle {
         let protocol = Box::leak(protocol);
 
         // Install a protocol interface for the load file protocol for the media loader protocol.
-        handle = unsafe {
+        let secondary_handle = unsafe {
             uefi::boot::install_protocol_interface(
-                Some(handle),
+                Some(primary_handle),
                 &LoadFile2Protocol::GUID,
-                protocol as *mut _ as *mut c_void,
+                // The UEFI API expects an opaque pointer here.
+                protocol as *mut MediaLoaderProtocol as *mut c_void,
             )
-        }
+        };
-        .context("unable to install media loader load file handle")?;
 
-        // Check if the media loader is registered.
+        // If installing the second protocol interface failed, we need to clean up after ourselves.
-        // If it is not, we can't continue safely because something went wrong.
+        if secondary_handle.is_err() {
-        if !Self::already_registered(guid)? {
+            // Uninstall the protocol interface for the device path protocol.
-            bail!("media loader not registered when expected to be registered");
+            // SAFETY: If we have reached this point, we know that the protocol is registered.
+            // If this fails, we have no choice but to leak memory. The error will be shown
+            // to the user, so at least they can see it. In most cases, catching this error
+            // will exit, so leaking is safe.
+            unsafe {
+                uefi::boot::uninstall_protocol_interface(
+                    primary_handle,
+                    &DevicePathProtocol::GUID,
+                    path.as_ffi_ptr() as *mut c_void,
+                )
+                .context(
+                    "unable to uninstall media loader device path handle, this will leak memory",
+                )?;
+            }
+
+            // SAFETY: We know that the protocol is leaked, so we can safely take a reference to it.
+            let protocol = unsafe { Box::from_raw(protocol) };
+            // SAFETY: We know that the data is leaked, so we can safely take a reference to it.
+            let data = unsafe { Box::from_raw(data) };
+            // SAFETY: We know that the path is leaked, so we can safely take a reference to it.
+            let path = unsafe { Box::from_raw(path) };
+
+            // Drop all the allocations explicitly to clarify the lifetime.
+            drop(protocol);
+            drop(data);
+            drop(path);
         }
 
+        // If installing the second protocol interface failed, this will return the error.
+        // We should have already cleaned up after ourselves, so this is safe.
+        secondary_handle.context("unable to install media loader load file handle")?;
+
         // Return a handle to the media loader.
         Ok(Self {
             guid,
-            handle,
+            handle: primary_handle,
             protocol,
             path,
         })