uefi/gop: fix memory leak

Author: phip1611

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## uefi - [Unreleased]
 
+### Changed
+- We fixed a memory leak in `GraphicsOutput::query_mode`. As a consequence, we
+  had to add `&BootServices` as additional parameter.
+
 ## uefi-macros - [Unreleased]
 
 ## uefi-raw - [Unreleased]

uefi-test-runner/src/proto/console/gop.rs

@@ -22,7 +22,7 @@ pub unsafe fn test(image: Handle, bt: &BootServices) {
         )
         .expect("failed to open Graphics Output Protocol");
 
-    set_graphics_mode(gop);
+    set_graphics_mode(gop, bt);
     fill_color(gop);
     draw_fb(gop);
 
@@ -33,10 +33,10 @@ pub unsafe fn test(image: Handle, bt: &BootServices) {
 }
 
 // Set a larger graphics mode.
-fn set_graphics_mode(gop: &mut GraphicsOutput) {
+fn set_graphics_mode(gop: &mut GraphicsOutput, bs: &BootServices) {
     // We know for sure QEMU has a 1024x768 mode.
     let mode = gop
-        .modes()
+        .modes(bs)
         .find(|mode| {
             let info = mode.info();
             info.resolution() == (1024, 768)

uefi/src/proto/console/gop.rs

@@ -50,6 +50,7 @@
 //! You will have to implement your own double buffering if you want to
 //! avoid tearing with animations.
 
+use crate::prelude::BootServices;
 use crate::proto::unsafe_protocol;
 use crate::util::usize_from_u32;
 use crate::{Result, StatusExt};
@@ -75,27 +76,37 @@ pub struct GraphicsOutput(GraphicsOutputProtocol);
 impl GraphicsOutput {
     /// Returns information for an available graphics mode that the graphics
     /// device and the set of active video output devices supports.
-    pub fn query_mode(&self, index: u32) -> Result<Mode> {
+    pub fn query_mode(&self, index: u32, bs: &BootServices) -> Result<Mode> {
         let mut info_sz = 0;
-        let mut info = ptr::null();
+        let mut info_heap_ptr = ptr::null();
+        // query_mode allocates a buffer and stores the heap ptr in the provided
+        // variable. In this buffer, the queried data can be found.
+        unsafe { (self.0.query_mode)(&self.0, index, &mut info_sz, &mut info_heap_ptr) }
+            .to_result_with_val(|| {
+                // Transform to owned info on the stack.
+                let info = unsafe { *info_heap_ptr };
+
+                let info_heap_ptr = info_heap_ptr.cast::<u8>().cast_mut();
+
+                // User has no benefit from propagating this error. If this
+                // fails, it is an error of the UEFI implementation.
+                bs.free_pool(info_heap_ptr)
+                    .expect("buffer should be deallocatable");
 
-        unsafe { (self.0.query_mode)(&self.0, index, &mut info_sz, &mut info) }.to_result_with_val(
-            || {
-                let info = unsafe { *info };
                 Mode {
                     index,
                     info_sz,
                     info: ModeInfo(info),
                 }
-            },
-        )
+            })
     }
 
     /// Returns information about all available graphics modes.
     #[must_use]
-    pub fn modes(&self) -> ModeIter {
+    pub fn modes<'a>(&'a self, bs: &'a BootServices) -> ModeIter {
         ModeIter {
             gop: self,
+            bs,
             current: 0,
             max: self.mode().max_mode,
         }
@@ -400,6 +411,7 @@ impl ModeInfo {
 /// Iterator for [`Mode`]s of the [`GraphicsOutput`] protocol.
 pub struct ModeIter<'gop> {
     gop: &'gop GraphicsOutput,
+    bs: &'gop BootServices,
     current: u32,
     max: u32,
 }
@@ -410,7 +422,7 @@ impl<'gop> Iterator for ModeIter<'gop> {
     fn next(&mut self) -> Option<Self::Item> {
         let index = self.current;
         if index < self.max {
-            let m = self.gop.query_mode(index);
+            let m = self.gop.query_mode(index, self.bs);
             self.current += 1;
 
             m.ok().or_else(|| self.next())