sprout: version 0.0.14

chore(deps): bump rustlang/rust from `141e9a7` to `7cba2ed` in the docker-updates group

Cargo.lock

@@ -116,7 +116,7 @@ dependencies = [
 
 [[package]]
 name = "edera-sprout"
-version = "0.0.12"
+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.12"
+version = "0.0.14"
 homepage = "https://sprout.edera.dev"
 repository = "https://github.com/edera-dev/sprout"
 edition = "2024"

Dockerfile

@@ -2,7 +2,7 @@
 ARG RUST_PROFILE=release
 ARG RUST_TARGET_SUBDIR=release
 
-FROM --platform=$BUILDPLATFORM rustlang/rust:nightly-alpine@sha256:141e9a7f13f77237dd4d462364c3a1b21cb8a6791d8924c409573e77b788af5e AS build
+FROM --platform=$BUILDPLATFORM rustlang/rust:nightly-alpine@sha256:7cba2edabb6ba0e92cd806cd1e0acae99d50f63e5b9c9ad842766d13c896d68c AS build
 RUN apk --no-cache add musl-dev busybox-static
 ARG RUST_PROFILE
 RUN adduser -S -s /bin/sh build

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.

docs/windows-setup.md

@@ -33,15 +33,10 @@ Write the following file to `X:\sprout.toml`:
 # sprout configuration: version 1
 version = 1
 
-# add a boot entry for booting Windows
+# global options.
-# which will run the boot-windows action.
+[options]
-[entries.windows]
+# enable autoconfiguration to detect Windows.
-title = "Windows"
+autoconfigure = true
-actions = ["boot-windows"]
-
-# use the chainload action to boot the Windows bootloader.
-[actions.boot-windows]
-chainload.path = "\\EFI\\Microsoft\\Boot\\bootmgfw.efi"
 ```
 
 ## Step 4: Configure EFI Firmware to boot Sprout

src/actions/chainload.rs

@@ -3,7 +3,7 @@ use crate::utils;
 use crate::utils::media_loader::MediaLoaderHandle;
 use crate::utils::media_loader::constants::linux::LINUX_EFI_INITRD_MEDIA_GUID;
 use anyhow::{Context, Result, bail};
-use log::{error, info};
+use log::error;
 use serde::{Deserialize, Serialize};
 use std::rc::Rc;
 use uefi::CString16;
@@ -69,8 +69,6 @@ pub fn chainload(context: Rc<SproutContext>, configuration: &ChainloadConfigurat
                 .context("unable to convert chainloader options to CString16")?,
         );
 
-        info!("options: {}", options);
-
         if options.num_bytes() > u32::MAX as usize {
             bail!("chainloader options too large");
         }
@@ -85,14 +83,17 @@ pub fn chainload(context: Rc<SproutContext>, configuration: &ChainloadConfigurat
         options_holder = Some(options);
     }
 
+    // Stamp the initrd path, if provided.
+    let initrd = configuration
+        .linux_initrd
+        .as_ref()
+        .map(|item| context.stamp(item));
     // The initrd can be None or empty, so we need to collapse that into a single Option.
-    let initrd = utils::empty_is_none(configuration.linux_initrd.as_ref());
+    let initrd = utils::empty_is_none(initrd);
 
     // If an initrd is provided, register it with the EFI stack.
     let mut initrd_handle = None;
     if let Some(linux_initrd) = initrd {
-        // Stamp the path to the initrd.
-        let linux_initrd = context.stamp(linux_initrd);
         let content = utils::read_file_contents(context.root().loaded_image_path()?, &linux_initrd)
             .context("unable to read linux initrd")?;
         let handle =

src/autoconfigure.rs

@@ -12,6 +12,9 @@ pub mod bls;
 /// on BLS-enabled filesystems as it may make duplicate entries.
 pub mod linux;
 
+/// windows: autodetect and configure Windows boot configurations.
+pub mod windows;
+
 /// Generate a [RootConfiguration] based on the environment.
 /// Intakes a `config` to use as the basis of the autoconfiguration.
 pub fn autoconfigure(config: &mut RootConfiguration) -> Result<()> {
@@ -44,6 +47,10 @@ pub fn autoconfigure(config: &mut RootConfiguration) -> Result<()> {
             linux::scan(&mut filesystem, &root, config)
                 .context("unable to scan for linux configurations")?;
         }
+
+        // Always look for Windows configurations.
+        windows::scan(&mut filesystem, &root, config)
+            .context("unable to scan for windows configurations")?;
     }
 
     Ok(())

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

@@ -23,7 +23,7 @@ const SCAN_LOCATIONS: &[&str] = &["/boot", "/"];
 const KERNEL_PREFIXES: &[&str] = &["vmlinuz"];
 
 /// Prefixes of initramfs files to match to.
-const INITRAMFS_PREFIXES: &[&str] = &["initramfs", "initrd"];
+const INITRAMFS_PREFIXES: &[&str] = &["initramfs", "initrd", "initrd.img"];
 
 /// Pair of kernel and initramfs.
 /// This is what scanning a directory is meant to find.
@@ -88,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;
             };
@@ -111,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 };
@@ -134,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.
@@ -158,7 +158,7 @@ pub fn scan(
     // Kernel pairs are detected, generate a list configuration for it.
     let generator = ListConfiguration {
         entry: EntryDeclaration {
-            title: "Boot Linux $kernel".to_string(),
+            title: "Boot Linux $name".to_string(),
             actions: vec![chainload_action_name.clone()],
             ..Default::default()
         },
@@ -166,8 +166,14 @@ pub fn scan(
             .into_iter()
             .map(|pair| {
                 BTreeMap::from_iter(vec![
-                    ("kernel".to_string(), pair.kernel),
+                    ("name".to_string(), pair.kernel.clone()),
-                    ("initrd".to_string(), pair.initramfs.unwrap_or_default()),
+                    ("kernel".to_string(), format!("{}{}", root, pair.kernel)),
+                    (
+                        "initrd".to_string(),
+                        pair.initramfs
+                            .map(|initramfs| format!("{}{}", root, initramfs))
+                            .unwrap_or_default(),
+                    ),
                 ])
             })
             .collect(),
@@ -194,9 +200,9 @@ pub fn scan(
     // Note that we don't need an extra \\ in the paths here.
     // The root already contains a trailing slash.
     let chainload = ChainloadConfiguration {
-        path: format!("{}$kernel", root),
+        path: "$kernel".to_string(),
         options: vec!["$linux-options".to_string()],
-        linux_initrd: Some(format!("{}$initrd", root)),
+        linux_initrd: Some("$initrd".to_string()),
     };
 
     // Insert the chainload action into the configuration.

src/autoconfigure/windows.rs

@@ -0,0 +1,80 @@
+use crate::actions::ActionDeclaration;
+use crate::actions::chainload::ChainloadConfiguration;
+use crate::config::RootConfiguration;
+use crate::entries::EntryDeclaration;
+use crate::utils;
+use anyhow::{Context, Result};
+use uefi::CString16;
+use uefi::fs::{FileSystem, Path};
+use uefi::proto::device_path::DevicePath;
+use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly};
+
+/// The name prefix of the Windows chainload action that will be used to boot Windows.
+const WINDOWS_CHAINLOAD_ACTION_PREFIX: &str = "windows-chainload-";
+
+/// Windows boot manager path.
+const BOOTMGR_FW_PATH: &str = "\\EFI\\Microsoft\\Boot\\bootmgfw.efi";
+
+/// Scan the specified `filesystem` for Windows configurations.
+pub fn scan(
+    filesystem: &mut FileSystem,
+    root: &DevicePath,
+    config: &mut RootConfiguration,
+) -> Result<bool> {
+    // Convert the boot manager firmware path to a path.
+    let bootmgr_fw_path =
+        CString16::try_from(BOOTMGR_FW_PATH).context("unable to convert path to CString16")?;
+    let bootmgr_fw_path = Path::new(&bootmgr_fw_path);
+
+    // Check if the boot manager firmware path exists, if it doesn't, return false.
+    if !filesystem
+        .try_exists(bootmgr_fw_path)
+        .context("unable to check if bootmgr firmware path exists")?
+    {
+        return Ok(false);
+    }
+
+    // Convert the device path root to a string we can use in the configuration.
+    let mut root = root
+        .to_string(DisplayOnly(false), AllowShortcuts(false))
+        .context("unable to convert device root to string")?
+        .to_string();
+    // 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.
+    let root_unique_hash = utils::unique_hash(&root);
+
+    // Generate a unique name for the Windows chainload action.
+    let chainload_action_name = format!("{}{}", WINDOWS_CHAINLOAD_ACTION_PREFIX, root_unique_hash,);
+
+    // Generate an entry name for Windows.
+    let entry_name = format!("autoconfigure-windows-{}", root_unique_hash,);
+
+    // Create an entry for Windows and insert it into the configuration.
+    let entry = EntryDeclaration {
+        title: "Boot Windows".to_string(),
+        actions: vec![chainload_action_name.clone()],
+        values: Default::default(),
+    };
+    config.entries.insert(entry_name, entry);
+
+    // Generate a chainload configuration for Windows.
+    let chainload = ChainloadConfiguration {
+        path: format!("{}{}", root, bootmgr_fw_path),
+        options: vec![],
+        ..Default::default()
+    };
+
+    // Insert the chainload action into the configuration.
+    config.actions.insert(
+        chainload_action_name,
+        ActionDeclaration {
+            chainload: Some(chainload),
+            ..Default::default()
+        },
+    );
+
+    // We have a Windows boot entry, so return true to indicate something was found.
+    Ok(true)
+}

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/main.rs

@@ -118,6 +118,9 @@ fn run() -> Result<()> {
 
         // Extend the root context with the autoconfigured actions.
         root.actions_mut().extend(config.actions);
+
+        // Insert any modified root values.
+        context.insert(&config.values);
     }
 
     // Refreeze the context to ensure that further operations can share the context.
@@ -248,6 +251,8 @@ fn main() -> Result<()> {
         for (index, stack) in error.chain().enumerate() {
             error!("[{}]: {}", index, stack);
         }
+        // Sleep for 10 seconds to allow the user to read the error.
+        uefi::boot::stall(Duration::from_secs(10));
     }
 
     // Sprout doesn't necessarily guarantee anything was booted.

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,
         })