Fix memory leaks in DevicePathFromText

The methods of both DevicePathToText and DevicePathFromText return memory that
is allocated internally by UEFI, so it needs to be freed on
drop. DevicePathToText already does that correctly by returning a `PoolString`,
but this was missed in DevicePathFromText, which just returns a reference.

Fix by adding PoolDevicePath and PoolDevicePathNode structs, and return them
from the corresponding methods of DevicePathFromText.

Author: nicholasbishop

uefi-test-runner/src/proto/device_path.rs

@@ -30,6 +30,17 @@ pub fn test() {
         )
         .expect("Failed to open DevicePathFromText protocol");
 
+        // Test round-trip conversion from path to text and back.
+        let device_path_string = device_path_to_text
+            .convert_device_path_to_text(&device_path, DisplayOnly(false), AllowShortcuts(false))
+            .unwrap();
+        assert_eq!(
+            *device_path_from_text
+                .convert_text_to_device_path(&device_path_string)
+                .unwrap(),
+            *device_path
+        );
+
         for path in device_path.node_iter() {
             info!(
                 "path: type={:?}, subtype={:?}, length={}",
@@ -47,7 +58,7 @@ pub fn test() {
             let convert = device_path_from_text
                 .convert_text_to_device_node(text)
                 .expect("Failed to convert text to device path");
-            assert_eq!(path, convert);
+            assert_eq!(*path, *convert);
         }
 
         // Get the `LoadedImageDevicePath`. Verify it start with the same nodes as

uefi/CHANGELOG.md

@@ -1,11 +1,18 @@
 # uefi - [Unreleased]
 
+## Added
+- Added `proto::device_path::PoolDevicePath` and
+  `proto::device_path::PoolDevicePathNode`.
+
 ## Changed
 - MSRV increased to 1.81.
 - `core::error::Error` impls are no longer gated by the `unstable` feature.
 - Fixed missing checks in the `TryFrom` conversion from `&DevicePathNode` to
   specific node types. The node type and subtype are now checked, and
   `NodeConversionError::DifferentType` is returned if they do not match.
+- **Breaking:** Fixed memory leaks in `DevicePathFromText` protocol. The methods
+  now return wrapper objects that free the device path / device path node on
+  drop.
 
 
 # uefi - 0.33.0 (2024-10-23)

uefi/src/mem/mod.rs

@@ -12,16 +12,17 @@ pub(crate) mod util;
 #[cfg(feature = "alloc")]
 pub(crate) use util::*;
 
-/// TODO
+/// Wrapper for memory allocated with UEFI's pool allocator. The memory is freed
+/// on drop.
 #[derive(Debug)]
 pub(crate) struct PoolAllocation(NonNull<u8>);
 
 impl PoolAllocation {
-    pub(crate) fn new(ptr: NonNull<u8>) -> Self {
+    pub(crate) const fn new(ptr: NonNull<u8>) -> Self {
         Self(ptr)
     }
 
-    pub(crate) fn as_ptr(&self) -> NonNull<u8> {
+    pub(crate) const fn as_ptr(&self) -> NonNull<u8> {
         self.0
     }
 }

uefi/src/proto/device_path/text.rs

@@ -61,6 +61,30 @@ impl Deref for PoolString {
     }
 }
 
+/// Device path allocated from UEFI pool memory.
+#[derive(Debug)]
+pub struct PoolDevicePath(PoolAllocation);
+
+impl Deref for PoolDevicePath {
+    type Target = DevicePath;
+
+    fn deref(&self) -> &Self::Target {
+        unsafe { DevicePath::from_ffi_ptr(self.0.as_ptr().as_ptr().cast()) }
+    }
+}
+
+/// Device path node allocated from UEFI pool memory.
+#[derive(Debug)]
+pub struct PoolDevicePathNode(PoolAllocation);
+
+impl Deref for PoolDevicePathNode {
+    type Target = DevicePathNode;
+
+    fn deref(&self) -> &Self::Target {
+        unsafe { DevicePathNode::from_ffi_ptr(self.0.as_ptr().as_ptr().cast()) }
+    }
+}
+
 /// Device Path to Text protocol.
 ///
 /// This protocol provides common utility functions for converting device
@@ -139,14 +163,12 @@ impl DevicePathFromText {
     pub fn convert_text_to_device_node(
         &self,
         text_device_node: &CStr16,
-    ) -> Result<&DevicePathNode> {
+    ) -> Result<PoolDevicePathNode> {
         unsafe {
             let ptr = (self.0.convert_text_to_device_node)(text_device_node.as_ptr().cast());
-            if ptr.is_null() {
-                Err(Status::OUT_OF_RESOURCES.into())
-            } else {
-                Ok(DevicePathNode::from_ffi_ptr(ptr.cast()))
-            }
+            NonNull::new(ptr.cast_mut())
+                .map(|p| PoolDevicePathNode(PoolAllocation::new(p.cast())))
+                .ok_or(Status::OUT_OF_RESOURCES.into())
         }
     }
 
@@ -160,14 +182,12 @@ impl DevicePathFromText {
     /// memory for the conversion.
     ///
     /// [`OUT_OF_RESOURCES`]: Status::OUT_OF_RESOURCES
-    pub fn convert_text_to_device_path(&self, text_device_path: &CStr16) -> Result<&DevicePath> {
+    pub fn convert_text_to_device_path(&self, text_device_path: &CStr16) -> Result<PoolDevicePath> {
         unsafe {
             let ptr = (self.0.convert_text_to_device_path)(text_device_path.as_ptr().cast());
-            if ptr.is_null() {
-                Err(Status::OUT_OF_RESOURCES.into())
-            } else {
-                Ok(DevicePath::from_ffi_ptr(ptr.cast()))
-            }
+            NonNull::new(ptr.cast_mut())
+                .map(|p| PoolDevicePath(PoolAllocation::new(p.cast())))
+                .ok_or(Status::OUT_OF_RESOURCES.into())
         }
     }
 }