diff --git a/CHANGELOG.md b/CHANGELOG.md index 253bcd0c4..f87092fe3 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/uefi-test-runner/src/proto/console/gop.rs b/uefi-test-runner/src/proto/console/gop.rs index c8c80e2c1..d0fd2946f 100644 --- a/uefi-test-runner/src/proto/console/gop.rs +++ b/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) diff --git a/uefi/src/proto/console/gop.rs b/uefi/src/proto/console/gop.rs index 1a60c2473..603e7ab14 100644 --- a/uefi/src/proto/console/gop.rs +++ b/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 { + pub fn query_mode(&self, index: u32, bs: &BootServices) -> Result { 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::().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 { 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())