Skip to content

Type-safety cleanup and safe exit from boot services #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Oct 23, 2018
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d873492
Clean up some lifetime usage in the GOP
HadrienG2 Oct 14, 2018
32351d3
Copy the GOP mode info to avoid use after free
HadrienG2 Oct 14, 2018
6fe2c08
Use a proper pointer for the File protocol
HadrienG2 Oct 14, 2018
6b9209c
Clean up usage of pointers/'static/usize + various tables
HadrienG2 Oct 14, 2018
0cd8ba8
Mark the configuration table const
HadrienG2 Oct 14, 2018
d963406
Do not directly expose 'static refs in the system table
HadrienG2 Oct 14, 2018
fb01aaa
Remove unnecessary feature gate for custom allocator
HadrienG2 Oct 14, 2018
d8faa60
Clarify allocation semantics
HadrienG2 Oct 14, 2018
892cbc4
Clarify concurrent protocol usage semantics
HadrienG2 Oct 14, 2018
74b8ae2
Prepare for a clean exit from UEFI boot services
HadrienG2 Oct 14, 2018
d9c5002
Interface create_event and run cargo fmt
HadrienG2 Oct 18, 2018
4291b33
Make UEFI utilities safe again
HadrienG2 Oct 18, 2018
073ca2e
A bit unsafe block makes more sense here
HadrienG2 Oct 18, 2018
8315e36
Fix my understanding of function pointers :)
HadrienG2 Oct 21, 2018
4780944
Introduce system table wrappers for safe boot service exit
HadrienG2 Oct 21, 2018
cf26597
Implement safe exit from boot services
HadrienG2 Oct 21, 2018
50d1e9b
Implement the RuntimeSystemTable
HadrienG2 Oct 21, 2018
6ef0bb3
Simplify call to mem::transmute
HadrienG2 Oct 21, 2018
907bf9b
Add an exit_boot_services demo
HadrienG2 Oct 21, 2018
f08f83a
Apply cargo fmt and clippy
HadrienG2 Oct 21, 2018
793dc7a
Follow suggestion from GabrielMajeri to avoid code duplication
HadrienG2 Oct 22, 2018
cd2db67
Fix incorrect Fn trait bounds
HadrienG2 Oct 22, 2018
99fbfef
Simplify the high-level exit_boot_services interface
HadrienG2 Oct 23, 2018
81f23da
Prefer a boxed slice over a vec
HadrienG2 Oct 23, 2018
ee4867e
Apply cargo fmt
HadrienG2 Oct 23, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion BUILDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ The following steps allow you to build a simple UEFI app.

```rust
#[no_mangle]
pub extern "win64" fn uefi_start(handle: Handle, system_table: &'static table::SystemTable) -> Status;
pub extern "win64" fn uefi_start(handle: Handle, system_table: SystemTable<Boot>) -> Status;
```

- Copy the `uefi-test-runner/x86_64-uefi.json` target file to your project's root.
Expand Down
2 changes: 1 addition & 1 deletion src/error/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<T> Completion<T> {
}

/// Transform the inner value without unwrapping the Completion
pub fn map<U>(self, f: impl Fn(T) -> U) -> Completion<U> {
pub fn map<U>(self, f: impl FnOnce(T) -> U) -> Completion<U> {
match self {
Completion::Success(res) => Completion::Success(f(res)),
Completion::Warning(res, stat) => Completion::Warning(f(res), stat),
Expand Down
4 changes: 2 additions & 2 deletions src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub trait ResultExt<T> {
fn expect_success(self, msg: &str) -> T;

/// Transform the inner output, if any
fn map_inner<U>(self, f: impl Fn(T) -> U) -> Result<U>;
fn map_inner<U>(self, f: impl FnOnce(T) -> U) -> Result<U>;
}

impl<T> ResultExt<T> for Result<T> {
Expand All @@ -49,7 +49,7 @@ impl<T> ResultExt<T> for Result<T> {
self.expect(msg).expect(msg)
}

fn map_inner<U>(self, f: impl Fn(T) -> U) -> Result<U> {
fn map_inner<U>(self, f: impl FnOnce(T) -> U) -> Result<U> {
self.map(|completion| completion.map(f))
}
}
4 changes: 3 additions & 1 deletion src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@
pub use crate::{ResultExt, Status};

// Import the basic table types.
pub use crate::table::{boot::BootServices, runtime::RuntimeServices, SystemTable};
pub use crate::table::boot::BootServices;
pub use crate::table::runtime::RuntimeServices;
pub use crate::table::{Boot, SystemTable};
43 changes: 26 additions & 17 deletions src/proto/console/gop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::{Completion, Result, Status};
/// The GOP can be used to set the properties of the frame buffer,
/// and also allows the app to access the in-memory buffer.
#[repr(C)]
pub struct GraphicsOutput {
pub struct GraphicsOutput<'boot> {
query_mode:
extern "win64" fn(&GraphicsOutput, mode: u32, info_sz: &mut usize, &mut *const ModeInfo)
-> Status,
Expand All @@ -42,7 +42,7 @@ pub struct GraphicsOutput {
#[allow(clippy::type_complexity)]
blt: extern "win64" fn(
this: &mut GraphicsOutput,
buffer: usize,
buffer: *mut BltPixel,
op: u32,
source_x: usize,
source_y: usize,
Expand All @@ -52,18 +52,18 @@ pub struct GraphicsOutput {
height: usize,
stride: usize,
) -> Status,
mode: &'static ModeData,
mode: &'boot ModeData<'boot>,
}

impl GraphicsOutput {
impl<'boot> GraphicsOutput<'boot> {
/// Returns information for an available graphics mode that the graphics
/// device and the set of active video output devices supports.
fn query_mode(&self, index: u32) -> Result<Mode> {
let mut info_sz = 0;
let mut info = ptr::null();

(self.query_mode)(self, index, &mut info_sz, &mut info).into_with(|| {
let info = unsafe { &*info };
let info = unsafe { *info };
Mode {
index,
info_sz,
Expand Down Expand Up @@ -103,7 +103,7 @@ impl GraphicsOutput {
self.check_framebuffer_region((dest_x, dest_y), (width, height));
(self.blt)(
self,
&color as *const _ as usize,
&color as *const _ as *mut _,
0,
0,
0,
Expand All @@ -126,7 +126,7 @@ impl GraphicsOutput {
match dest_region {
BltRegion::Full => (self.blt)(
self,
buffer.as_mut_ptr() as usize,
buffer.as_mut_ptr(),
1,
src_x,
src_y,
Expand All @@ -142,7 +142,7 @@ impl GraphicsOutput {
px_stride,
} => (self.blt)(
self,
buffer.as_mut_ptr() as usize,
buffer.as_mut_ptr(),
1,
src_x,
src_y,
Expand All @@ -166,7 +166,7 @@ impl GraphicsOutput {
match src_region {
BltRegion::Full => (self.blt)(
self,
buffer.as_ptr() as usize,
buffer.as_ptr() as *mut _,
2,
0,
0,
Expand All @@ -182,7 +182,7 @@ impl GraphicsOutput {
px_stride,
} => (self.blt)(
self,
buffer.as_ptr() as usize,
buffer.as_ptr() as *mut _,
2,
src_x,
src_y,
Expand All @@ -203,7 +203,16 @@ impl GraphicsOutput {
self.check_framebuffer_region((src_x, src_y), (width, height));
self.check_framebuffer_region((dest_x, dest_y), (width, height));
(self.blt)(
self, 0usize, 3, src_x, src_y, dest_x, dest_y, width, height, 0,
self,
ptr::null_mut(),
3,
src_x,
src_y,
dest_x,
dest_y,
width,
height,
0,
)
.into()
}
Expand Down Expand Up @@ -269,19 +278,19 @@ impl GraphicsOutput {
}

impl_proto! {
protocol GraphicsOutput {
protocol GraphicsOutput<'boot> {
GUID = 0x9042a9de, 0x23dc, 0x4a38, [0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a];
}
}

#[repr(C)]
struct ModeData {
struct ModeData<'a> {
// Number of modes which the GOP supports.
max_mode: u32,
// Current mode.
mode: u32,
// Information about the current mode.
info: &'static ModeInfo,
info: &'a ModeInfo,
// Size of the above structure.
info_sz: usize,
// Physical address of the frame buffer.
Expand Down Expand Up @@ -329,7 +338,7 @@ pub struct PixelBitmask {
pub struct Mode {
index: u32,
info_sz: usize,
info: &'static ModeInfo,
info: ModeInfo,
}

impl Mode {
Expand All @@ -342,7 +351,7 @@ impl Mode {

/// Returns a reference to the mode info structure.
pub fn info(&self) -> &ModeInfo {
self.info
&self.info
}
}

Expand Down Expand Up @@ -391,7 +400,7 @@ impl ModeInfo {

/// Iterator for graphics modes.
struct ModeIter<'a> {
gop: &'a GraphicsOutput,
gop: &'a GraphicsOutput<'a>,
current: u32,
max: u32,
}
Expand Down
8 changes: 4 additions & 4 deletions src/proto/console/pointer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use crate::{Event, Result, Status};

/// Provides information about a pointer device.
#[repr(C)]
pub struct Pointer {
pub struct Pointer<'boot> {
reset: extern "win64" fn(this: &mut Pointer, ext_verif: bool) -> Status,
get_state: extern "win64" fn(this: &Pointer, state: &mut PointerState) -> Status,
wait_for_input: Event,
mode: &'static PointerMode,
mode: &'boot PointerMode,
}

impl Pointer {
impl<'boot> Pointer<'boot> {
/// Resets the pointer device hardware.
///
/// The `extended_verification` parameter is used to request that UEFI
Expand Down Expand Up @@ -55,7 +55,7 @@ impl Pointer {
}

impl_proto! {
protocol Pointer {
protocol Pointer<'boot> {
GUID = 0x31878c87, 0xb75, 0x11d5, [0x9a, 0x4f, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d];
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/proto/console/serial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{Completion, Result, Status};
/// Since UEFI drivers are implemented through polling, if you fail to regularly
/// check for input/output, some data might be lost.
#[repr(C)]
pub struct Serial {
pub struct Serial<'boot> {
// Revision of this protocol, only 1.0 is currently defined.
// Future versions will be backwards compatible.
revision: u32,
Expand All @@ -29,10 +29,10 @@ pub struct Serial {
get_control_bits: extern "win64" fn(&Serial, &mut ControlBits) -> Status,
write: extern "win64" fn(&mut Serial, &mut usize, *const u8) -> Status,
read: extern "win64" fn(&mut Serial, &mut usize, *mut u8) -> Status,
io_mode: &'static IoMode,
io_mode: &'boot IoMode,
}

impl Serial {
impl<'boot> Serial<'boot> {
/// Reset the device.
pub fn reset(&mut self) -> Result<()> {
(self.reset)(self).into()
Expand Down Expand Up @@ -117,7 +117,7 @@ impl Serial {
}

impl_proto! {
protocol Serial {
protocol Serial<'boot> {
GUID = 0xBB25CF6F, 0xF1D4, 0x11D2, [0x9A, 0x0C, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0xFD];
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/proto/console/text/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{CStr16, Char16, Completion, Result, Status};
/// It implements the fmt::Write trait, so you can use it to print text with
/// standard Rust constructs like the write!() and writeln!() macros.
#[repr(C)]
pub struct Output {
pub struct Output<'boot> {
reset: extern "win64" fn(this: &Output, extended: bool) -> Status,
output_string: extern "win64" fn(this: &Output, string: *const Char16) -> Status,
test_string: extern "win64" fn(this: &Output, string: *const Char16) -> Status,
Expand All @@ -18,10 +18,10 @@ pub struct Output {
clear_screen: extern "win64" fn(this: &mut Output) -> Status,
set_cursor_position: extern "win64" fn(this: &mut Output, column: usize, row: usize) -> Status,
enable_cursor: extern "win64" fn(this: &mut Output, visible: bool) -> Status,
data: &'static OutputData,
data: &'boot OutputData,
}

impl Output {
impl<'boot> Output<'boot> {
/// Resets and clears the text output device hardware.
pub fn reset(&mut self, extended: bool) -> Result<()> {
(self.reset)(self, extended).into()
Expand Down Expand Up @@ -53,8 +53,8 @@ impl Output {
}

/// Returns an iterator of all supported text modes.
// TODO: fix the ugly lifetime parameter.
pub fn modes<'a>(&'a mut self) -> impl Iterator<Item = Completion<OutputMode>> + 'a {
// TODO: Bring back impl Trait once the story around bounds improves
pub fn modes<'a>(&'a mut self) -> OutputModeIter<'a, 'boot> {
let max = self.data.max_mode;
OutputModeIter {
output: self,
Expand Down Expand Up @@ -134,7 +134,7 @@ impl Output {
}
}

impl fmt::Write for Output {
impl<'boot> fmt::Write for Output<'boot> {
fn write_str(&mut self, s: &str) -> fmt::Result {
// Allocate a small buffer on the stack.
const BUF_SIZE: usize = 128;
Expand Down Expand Up @@ -215,13 +215,13 @@ impl OutputMode {
}

/// An iterator of the text modes (possibly) supported by a device.
struct OutputModeIter<'a> {
output: &'a mut Output,
pub struct OutputModeIter<'a, 'b: 'a> {
output: &'a mut Output<'b>,
current: i32,
max: i32,
}

impl<'a> Iterator for OutputModeIter<'a> {
impl<'a, 'b> Iterator for OutputModeIter<'a, 'b> {
type Item = Completion<OutputMode>;

fn next(&mut self) -> Option<Self::Item> {
Expand Down Expand Up @@ -259,7 +259,7 @@ struct OutputData {
}

impl_proto! {
protocol Output {
protocol Output<'boot> {
GUID = 0x387477c2, 0x69c7, 0x11d2, [0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b];
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/proto/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,21 @@ macro_rules! impl_proto {
// Most UEFI functions do not support multithreaded access.
impl !Sync for $p {}
};
(
protocol $p:ident<'boot> {
GUID = $a:expr, $b:expr, $c:expr, $d:expr;
}
) => {
impl<'boot> $crate::proto::Protocol for $p<'boot> {
#[doc(hidden)]
// These literals aren't meant to be human-readable.
#[allow(clippy::unreadable_literal)]
const GUID: $crate::Guid = $crate::Guid::from_values($a, $b, $c, $d);
}

// Most UEFI functions expect to be called on the bootstrap processor.
impl<'boot> !Send for $p<'boot> {}
// Most UEFI functions do not support multithreaded access.
impl<'boot> !Sync for $p<'boot> {}
};
}
17 changes: 7 additions & 10 deletions src/proto/media/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use bitflags::bitflags;
use core::mem;
use core::ptr;
use crate::prelude::*;
use crate::{CStr16, Char16, Result, Status};
use ucs2;
Expand All @@ -22,10 +23,8 @@ pub struct File<'a> {
}

impl<'a> File<'a> {
pub(super) unsafe fn new(ptr: usize) -> Self {
let ptr = ptr as *mut FileImpl;
let inner = &mut *ptr;
File { inner }
pub(super) unsafe fn new(ptr: *mut FileImpl) -> Self {
File { inner: &mut *ptr }
}

/// Try to open a file relative to this file/directory.
Expand Down Expand Up @@ -58,7 +57,7 @@ impl<'a> File<'a> {
Err(Status::INVALID_PARAMETER)
} else {
let mut buf = [0u16; BUF_SIZE + 1];
let mut ptr = 0usize;
let mut ptr = ptr::null_mut();

let len = ucs2::encode(filename, &mut buf)?;
let filename = unsafe { CStr16::from_u16_with_nul_unchecked(&buf[..=len]) };
Expand All @@ -70,9 +69,7 @@ impl<'a> File<'a> {
open_mode,
attributes,
)
.into_with(|| File {
inner: unsafe { &mut *(ptr as *mut FileImpl) },
})
.into_with(|| unsafe { File::new(ptr) })
}
}

Expand Down Expand Up @@ -175,11 +172,11 @@ impl<'a> Drop for File<'a> {

/// The function pointer table for the File protocol.
#[repr(C)]
struct FileImpl {
pub(super) struct FileImpl {
revision: u64,
open: extern "win64" fn(
this: &mut FileImpl,
new_handle: &mut usize,
new_handle: &mut *mut FileImpl,
filename: *const Char16,
open_mode: FileMode,
attributes: FileAttribute,
Expand Down
Loading