Skip to content

Commit b7e2fff

Browse files
necauquanicholasbishop
authored andcommitted
Change Handle repr to be non-nullable so that Option<Handle> is ffi-safe
1 parent 452b9c6 commit b7e2fff

File tree

3 files changed

+34
-24
lines changed

3 files changed

+34
-24
lines changed

src/data_types/mod.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,14 @@
22
//!
33
//! This module defines the basic data types that are used throughout uefi-rs
44
5-
use core::{ffi::c_void, mem::MaybeUninit};
5+
use core::{ffi::c_void, ptr::NonNull};
66

7-
/// Opaque handle to an UEFI entity (protocol, image...)
7+
/// Opaque handle to an UEFI entity (protocol, image...), guaranteed to be non-null.
8+
///
9+
/// If you need to have a nullable handle (for a custom UEFI FFI for example) use `Option<Handle>`.
810
#[derive(Clone, Copy, Debug)]
911
#[repr(transparent)]
10-
pub struct Handle(*mut c_void);
11-
12-
impl Handle {
13-
pub(crate) unsafe fn uninitialized() -> Self {
14-
MaybeUninit::zeroed().assume_init()
15-
}
16-
}
12+
pub struct Handle(NonNull<c_void>);
1713

1814
/// Handle to an event structure
1915
#[repr(transparent)]

src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@
2323
//! For example, a PC with no network card might not contain a network driver,
2424
//! therefore all the network protocols will be unavailable.
2525
26-
#![cfg_attr(feature = "exts", feature(allocator_api, alloc_layout_extra))]
26+
#![cfg_attr(
27+
feature = "exts",
28+
feature(allocator_api, alloc_layout_extra, vec_into_raw_parts)
29+
)]
2730
#![feature(auto_traits)]
2831
#![feature(control_flow_enum)]
2932
#![feature(try_trait_v2)]

src/table/boot.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,12 @@ pub struct BootServices {
7575
proto: *const Guid,
7676
key: *mut c_void,
7777
buf_sz: &mut usize,
78-
buf: *mut Handle,
78+
buf: *mut MaybeUninit<Handle>,
7979
) -> Status,
8080
locate_device_path: unsafe extern "efiapi" fn(
8181
proto: &Guid,
8282
device_path: &mut &DevicePath,
83-
out_handle: *mut Handle,
83+
out_handle: &mut MaybeUninit<Handle>,
8484
) -> Status,
8585
install_configuration_table: usize,
8686

@@ -91,7 +91,7 @@ pub struct BootServices {
9191
device_path: *const DevicePath,
9292
source_buffer: *const u8,
9393
source_size: usize,
94-
*mut Handle,
94+
image_handle: &mut MaybeUninit<Handle>,
9595
) -> Status,
9696
start_image: unsafe extern "efiapi" fn(
9797
image_handle: Handle,
@@ -502,11 +502,11 @@ impl BootServices {
502502
pub fn locate_handle(
503503
&self,
504504
search_ty: SearchType,
505-
output: Option<&mut [Handle]>,
505+
output: Option<&mut [MaybeUninit<Handle>]>,
506506
) -> Result<usize> {
507507
let handle_size = mem::size_of::<Handle>();
508508

509-
const NULL_BUFFER: *mut Handle = ptr::null_mut();
509+
const NULL_BUFFER: *mut MaybeUninit<Handle> = ptr::null_mut();
510510

511511
let (mut buffer_size, buffer) = match output {
512512
Some(buffer) => (buffer.len() * handle_size, buffer.as_mut_ptr()),
@@ -541,9 +541,10 @@ impl BootServices {
541541
/// protocol, the `device_path` is advanced to the device path terminator node. If `device_path`
542542
/// is a multi-instance device path, the function will operate on the first instance.
543543
pub fn locate_device_path<P: Protocol>(&self, device_path: &mut &DevicePath) -> Result<Handle> {
544+
let mut handle = MaybeUninit::uninit();
544545
unsafe {
545-
let mut handle = Handle::uninitialized();
546-
(self.locate_device_path)(&P::GUID, device_path, &mut handle).into_with_val(|| handle)
546+
(self.locate_device_path)(&P::GUID, device_path, &mut handle)
547+
.into_with_val(|| handle.assume_init())
547548
}
548549
}
549550

@@ -553,11 +554,11 @@ impl BootServices {
553554
parent_image_handle: Handle,
554555
source_buffer: &[u8],
555556
) -> Result<Handle> {
557+
let boot_policy = 0;
558+
let device_path = ptr::null();
559+
let source_size = source_buffer.len();
560+
let mut image_handle = MaybeUninit::uninit();
556561
unsafe {
557-
let boot_policy = 0;
558-
let device_path = ptr::null();
559-
let source_size = source_buffer.len();
560-
let mut image_handle = Handle::uninitialized();
561562
(self.load_image)(
562563
boot_policy,
563564
parent_image_handle,
@@ -566,7 +567,7 @@ impl BootServices {
566567
source_size,
567568
&mut image_handle,
568569
)
569-
.into_with_val(|| image_handle)
570+
.into_with_val(|| image_handle.assume_init())
570571
}
571572
}
572573

@@ -745,7 +746,7 @@ impl BootServices {
745746

746747
// Allocate a large enough buffer.
747748
let mut buffer = Vec::new();
748-
buffer.resize_with(buffer_size, || unsafe { Handle::uninitialized() });
749+
buffer.resize_with(buffer_size, MaybeUninit::uninit);
749750

750751
// Perform the search.
751752
let (status2, buffer_size) = self.locate_handle(search_type, Some(&mut buffer))?.split();
@@ -759,9 +760,19 @@ impl BootServices {
759760
// `Status::BUFFER_TOO_SMALL` and `buffer` will be dropped.
760761
buffer.truncate(buffer_size);
761762

763+
// Convert the buffer from MaybeUninits to Handles.
764+
// The raw parts roundtrip with a pointer cast is better than just
765+
// transmuting vectors per mem::transmute docs.
766+
// Transmuting MaybeUninit<T> to T is also correct, if we are sure that
767+
// it is initialized, which it is, unless UEFI is broken
768+
let handles = unsafe {
769+
let (ptr, len, cap) = buffer.into_raw_parts();
770+
Vec::from_raw_parts(ptr as *mut Handle, len, cap)
771+
};
772+
762773
// Emit output, with warnings
763774
status1
764-
.into_with_val(|| buffer)
775+
.into_with_val(|| handles)
765776
.map(|completion| completion.with_status(status2))
766777
}
767778

0 commit comments

Comments
 (0)