Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit eb883da

Browse files
End of this pass of uefi-rs review (#74)
* Fix a forgotten method * Minor adjustments * Clarify the safety requirements of raise/restore_tpl, use RAII * Clarify safety of get_memory_map * Clean up create_event * Document TplGuard * Mark wait_for_event as unsafe * Clarify contract of Handle-related functions * ExitBootServices is _very_ unsafe... * Clarify set_watchdog_timer contract * Mark memory ops as unsafe * Record presence of UB in exit_boot_services * Simplify page allocator test * Homogeneize memory map margins * Typo fix
1 parent 0d0966f commit eb883da

File tree

6 files changed

+85
-59
lines changed

6 files changed

+85
-59
lines changed

‎src/table/boot.rs

Lines changed: 75 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ pub struct BootServices {
1414
header: Header,
1515

1616
// Task Priority services
17-
raise_tpl: extern "win64" fn(new_tpl: Tpl) -> Tpl,
18-
restore_tpl: extern "win64" fn(old_tpl: Tpl),
17+
raise_tpl: unsafeextern "win64" fn(new_tpl: Tpl) -> Tpl,
18+
restore_tpl: unsafeextern "win64" fn(old_tpl: Tpl),
1919

2020
// Memory allocation functions
2121
allocate_pages: extern "win64" fn(
@@ -25,7 +25,7 @@ pub struct BootServices {
2525
addr: &mut u64,
2626
) -> Status,
2727
free_pages: extern "win64" fn(addr: u64, pages: usize) -> Status,
28-
memory_map: extern "win64" fn(
28+
get_memory_map:unsafe extern "win64" fn(
2929
size: &mut usize,
3030
map: *mut MemoryDescriptor,
3131
key: &mut MemoryMapKey,
@@ -37,15 +37,15 @@ pub struct BootServices {
3737
free_pool: extern "win64" fn(buffer: *mut u8) -> Status,
3838

3939
// Event & timer functions
40-
create_event: extern "win64" fn(
40+
create_event: unsafeextern "win64" fn(
4141
ty: EventType,
4242
notify_tpl: Tpl,
4343
notify_func: Option<EventNotifyFn>,
4444
notify_ctx: *mut c_void,
45-
event: *mut Event,
45+
event: &mut Event,
4646
) -> Status,
4747
set_timer: usize,
48-
wait_for_event: extern "win64" fn(
48+
wait_for_event: unsafeextern "win64" fn(
4949
number_of_events: usize,
5050
events: *mut Event,
5151
out_index: &mut usize,
@@ -58,14 +58,11 @@ pub struct BootServices {
5858
install_protocol_interface: usize,
5959
reinstall_protocol_interface: usize,
6060
uninstall_protocol_interface: usize,
61-
handle_protocol: extern "win64" fn(
62-
handle: Handle,
63-
proto: *const Guid,
64-
out_proto: &mut *mut c_void,
65-
) -> Status,
61+
handle_protocol:
62+
extern "win64" fn(handle: Handle, proto: &Guid, out_proto: &mut *mut c_void) -> Status,
6663
_reserved: usize,
6764
register_protocol_notify: usize,
68-
locate_handle: extern "win64" fn(
65+
locate_handle: unsafeextern "win64" fn(
6966
search_ty: i32,
7067
proto: *const Guid,
7168
key: *mut c_void,
@@ -80,12 +77,13 @@ pub struct BootServices {
8077
start_image: usize,
8178
exit: usize,
8279
unload_image: usize,
83-
exit_boot_services: extern "win64" fn(image_handle: Handle, map_key: MemoryMapKey) -> Status,
80+
exit_boot_services:
81+
unsafe extern "win64" fn(image_handle: Handle, map_key: MemoryMapKey) -> Status,
8482

8583
// Misc services
8684
get_next_monotonic_count: usize,
8785
stall: extern "win64" fn(microseconds: usize) -> Status,
88-
set_watchdog_timer: extern "win64" fn(
86+
set_watchdog_timer: unsafeextern "win64" fn(
8987
timeout: usize,
9088
watchdog_code: u64,
9189
data_size: usize,
@@ -112,22 +110,27 @@ pub struct BootServices {
112110
calculate_crc32: usize,
113111

114112
// Misc services
115-
copy_mem: extern "win64" fn(dest: *mut u8, src: *const u8, len: usize),
116-
set_mem: extern "win64" fn(buffer: *mut u8, len: usize, value: u8),
113+
copy_mem: unsafeextern "win64" fn(dest: *mut u8, src: *const u8, len: usize),
114+
set_mem: unsafeextern "win64" fn(buffer: *mut u8, len: usize, value: u8),
117115

118116
// New event functions (UEFI 2.0 or newer)
119117
create_event_ex: usize,
120118
}
121119

122120
impl BootServices {
123121
/// Raises a task's priority level and returns its previous level.
124-
pub fn raise_tpl(&self, tpl: Tpl) -> Tpl {
125-
(self.raise_tpl)(tpl)
126-
}
127-
128-
/// Restores a task’s priority level to its previous value.
129-
pub fn restore_tpl(&self, old_tpl: Tpl) {
130-
(self.restore_tpl)(old_tpl)
122+
///
123+
/// The effect of calling raise_tpl with a Tpl that is below the current one
124+
/// (which, sadly, cannot be queried) is undefined by the UEFI spec, which
125+
/// also warns against remaining at high Tpls for extended periods of time.
126+
///
127+
/// This function outputs an RAII guard that will automatically restore the
128+
/// original Tpl when dropped.
129+
pub unsafe fn raise_tpl(&self, tpl: Tpl) -> TplGuard<'_> {
130+
TplGuard {
131+
boot_services: self,
132+
old_tpl: (self.raise_tpl)(tpl),
133+
}
131134
}
132135

133136
/// Allocates memory pages from the system.
@@ -165,13 +168,15 @@ impl BootServices {
165168
let mut entry_size = 0;
166169
let mut entry_version = 0;
167170

168-
let status = (self.memory_map)(
169-
&mut map_size,
170-
ptr::null_mut(),
171-
&mut map_key,
172-
&mut entry_size,
173-
&mut entry_version,
174-
);
171+
let status = unsafe {
172+
(self.get_memory_map)(
173+
&mut map_size,
174+
ptr::null_mut(),
175+
&mut map_key,
176+
&mut entry_size,
177+
&mut entry_version,
178+
)
179+
};
175180
assert_eq!(status, Status::BUFFER_TOO_SMALL);
176181

177182
map_size * entry_size
@@ -184,6 +189,8 @@ impl BootServices {
184189
///
185190
/// The returned key is a unique identifier of the current configuration of memory.
186191
/// Any allocations or such will change the memory map's key.
192+
///
193+
/// FIXME: The input buffer must be properly aligned or UB will occur.
187194
pub fn memory_map<'a>(
188195
&self,
189196
buffer: &'a mut [u8],
@@ -194,13 +201,15 @@ impl BootServices {
194201
let mut entry_size = 0;
195202
let mut entry_version = 0;
196203

197-
(self.memory_map)(
198-
&mut map_size,
199-
map_buffer,
200-
&mut map_key,
201-
&mut entry_size,
202-
&mut entry_version,
203-
)
204+
unsafe {
205+
(self.get_memory_map)(
206+
&mut map_size,
207+
map_buffer,
208+
&mut map_key,
209+
&mut entry_size,
210+
&mut entry_version,
211+
)
212+
}
204213
.into_with(move || {
205214
let len = map_size / entry_size;
206215
let iter = MemoryMapIter {
@@ -245,7 +254,7 @@ impl BootServices {
245254

246255
// Use a trampoline to handle the impedance mismatch between Rust & C
247256
unsafe extern "win64" fn notify_trampoline(e: Event, ctx: *mut c_void) {
248-
let notify_fn: fn(Event) = core::mem::transmute(ctx);
257+
let notify_fn: fn(Event) = mem::transmute(ctx);
249258
notify_fn(e); // SAFETY: Aborting panics are assumed here
250259
}
251260
let (notify_func, notify_ctx) = notify_fn
@@ -258,7 +267,7 @@ impl BootServices {
258267
.unwrap_or((None, ptr::null_mut()));
259268

260269
// Now we're ready to call UEFI
261-
(self.create_event)(event_ty, notify_tpl, notify_func, notify_ctx, &mut event)
270+
unsafe{(self.create_event)(event_ty, notify_tpl, notify_func, notify_ctx, &mut event)}
262271
.into_with(|| event)
263272
}
264273

@@ -292,7 +301,7 @@ impl BootServices {
292301
pub fn wait_for_event(&self, events: &mut [Event]) -> result::Result<usize, (Status, usize)> {
293302
let (number_of_events, events) = (events.len(), events.as_mut_ptr());
294303
let mut index = unsafe { mem::uninitialized() };
295-
match (self.wait_for_event)(number_of_events, events, &mut index) {
304+
match unsafe{(self.wait_for_event)(number_of_events, events, &mut index)} {
296305
Status::SUCCESS => Ok(index),
297306
s @ Status::INVALID_PARAMETER => Err((s, index)),
298307
error => Err((error, 0)),
@@ -344,7 +353,7 @@ impl BootServices {
344353
SearchType::ByProtocol(guid) => (2, guid as *const _, ptr::null_mut()),
345354
};
346355

347-
let status = (self.locate_handle)(ty, guid, key, &mut buffer_size, buffer);
356+
let status = unsafe{(self.locate_handle)(ty, guid, key, &mut buffer_size, buffer)};
348357

349358
// Must convert the returned size (in bytes) to length (number of elements).
350359
let buffer_len = buffer_size / handle_size;
@@ -394,8 +403,7 @@ impl BootServices {
394403
/// allows you to change what will be logged when the timer expires.
395404
///
396405
/// The watchdog codes from 0 to 0xffff (65535) are reserved for internal
397-
/// firmware use. You should therefore only use them if instructed to do so
398-
/// by firmware-specific documentation. Higher values can be used freely.
406+
/// firmware use. Higher values can be used freely by applications.
399407
///
400408
/// If provided, the watchdog data must be a null-terminated string
401409
/// optionally followed by other binary data.
@@ -405,24 +413,28 @@ impl BootServices {
405413
watchdog_code: u64,
406414
data: Option<&mut [u16]>,
407415
) -> Result<()> {
416+
assert!(
417+
watchdog_code > 0xffff,
418+
"Invalid use of a reserved firmware watchdog code"
419+
);
420+
408421
let (data_len, data) = data
409422
.map(|d| {
410423
assert!(
411424
d.contains(&0),
412-
"Watchdog data must contain a null-terminated string"
425+
"Watchdog data must start with a null-terminated string"
413426
);
414427
(d.len(), d.as_mut_ptr())
415428
})
416429
.unwrap_or((0, ptr::null_mut()));
417430

418-
(self.set_watchdog_timer)(timeout, watchdog_code, data_len, data).into()
431+
unsafe{(self.set_watchdog_timer)(timeout, watchdog_code, data_len, data)}.into()
419432
}
420433

421434
/// Copies memory from source to destination. The buffers can overlap.
422435
///
423436
/// This function is unsafe as it can be used to violate most safety
424437
/// invariants of the Rust type system.
425-
///
426438
pub unsafe fn memmove(&self, dest: *mut u8, src: *const u8, size: usize) {
427439
(self.copy_mem)(dest, src, size);
428440
}
@@ -431,7 +443,6 @@ impl BootServices {
431443
///
432444
/// This function is unsafe as it can be used to violate most safety
433445
/// invariants of the Rust type system.
434-
///
435446
pub unsafe fn memset(&self, buffer: *mut u8, size: usize, value: u8) {
436447
(self.set_mem)(buffer, size, value);
437448
}
@@ -466,6 +477,22 @@ pub enum Tpl: usize => {
466477
HIGH_LEVEL = 31,
467478
}}
468479

480+
/// RAII guard for task priority level changes
481+
///
482+
/// Will automatically restore the former task priority level when dropped.
483+
pub struct TplGuard<'a> {
484+
boot_services: &'a BootServices,
485+
old_tpl: Tpl,
486+
}
487+
488+
impl Drop for TplGuard<'_> {
489+
fn drop(&mut self) {
490+
unsafe {
491+
(self.boot_services.restore_tpl)(self.old_tpl);
492+
}
493+
}
494+
}
495+
469496
/// Type of allocation to perform.
470497
#[derive(Debug, Copy, Clone)]
471498
pub enum AllocateType {
@@ -620,7 +647,7 @@ impl<'a> Iterator for MemoryMapIter<'a> {
620647

621648
self.index += 1;
622649

623-
let descriptor:&MemoryDescriptor= unsafe { mem::transmute(ptr) };
650+
let descriptor= unsafe { &*(ptras*constMemoryDescriptor) };
624651

625652
Some(descriptor)
626653
} else {

‎src/table/system.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ impl SystemTable<Boot> {
129129
/// system table which more accurately reflects the state of the UEFI
130130
/// firmware following exit from boot services, along with a high-level
131131
/// iterator to the UEFI memory map.
132+
///
133+
/// FIXME: UB will occur if mmap_buf is not well aligned
132134
pub fn exit_boot_services<'a>(
133135
self,
134136
image: Handle,

‎uefi-logger/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ impl Logger {
5252

5353
impl<'boot> log::Log for Logger {
5454
fn enabled(&self, _metadata: &log::Metadata) -> bool {
55-
true
55+
self.writer.is_some()
5656
}
5757

5858
fn log(&self, record: &log::Record) {

‎uefi-test-runner/src/boot/memory.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,8 @@ fn allocate_pages(bt: &BootServices) {
2525

2626
assert_eq!(pgs % 4096, 0, "Page pointer is not page-aligned");
2727

28-
// Simple page structure to test this code.
29-
#[repr(C, align(4096))]
30-
struct Page([u8; 4096]);
31-
32-
let page: &Page = unsafe { mem::transmute(pgs) };
33-
34-
let mut buf = page.0;
28+
// Reinterprete the page as an array of bytes
29+
let buf = unsafe { &mut *(pgs as *mut [u8; 4096]) };
3530

3631
// If these don't fail then we properly allocated some memory.
3732
buf[0] = 0xF0;

‎uefi-test-runner/src/main.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ extern crate log;
1010
#[macro_use]
1111
extern crate alloc;
1212

13+
use core::mem;
1314
use uefi::prelude::*;
1415
use uefi::proto::console::serial::Serial;
16+
use uefi::table::boot::MemoryDescriptor;
1517
use uefi_exts::BootServicesExt;
1618

1719
mod boot;
@@ -61,7 +63,6 @@ fn check_revision(rev: uefi::table::Revision) {
6163
/// This functionality is very specific to our QEMU-based test runner. Outside
6264
/// of it, we just pause the tests for a couple of seconds to allow visual
6365
/// inspection of the output.
64-
///
6566
fn check_screenshot(bt: &BootServices, name: &str) {
6667
if cfg!(feature = "qemu") {
6768
// Access the serial port (in a QEMU environment, it should always be there)
@@ -115,7 +116,8 @@ fn shutdown(image: uefi::Handle, st: SystemTable<Boot>) -> ! {
115116
}
116117

117118
// Exit boot services as a proof that it works :)
118-
let max_mmap_size = st.boot_services().memory_map_size() + 1024;
119+
let max_mmap_size =
120+
st.boot_services().memory_map_size() + 8 * mem::size_of::<MemoryDescriptor>();
119121
let mut mmap_storage = vec![0; max_mmap_size].into_boxed_slice();
120122
let (st, _iter) = st
121123
.exit_boot_services(image, &mut mmap_storage[..])

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub fn test(bt: &BootServices) {
2121

2222
// Set a larger graphics mode.
2323
fn set_graphics_mode(gop: &mut GraphicsOutput) {
24-
// We know for sure QEMU has a 1024x768, mode.
24+
// We know for sure QEMU has a 1024x768 mode.
2525
let mode = gop
2626
.modes()
2727
.map(|mode| mode.expect("Warnings encountered while querying mode"))

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /