rust-osdev / uefi-rs

Rusty wrapper for the Unified Extensible Firmware Interface (UEFI). This crate makes it easy to develop Rust software that leverages safe, convenient, and performant abstractions for UEFI functionality.
https://rust-osdev.com/uefi-book
Mozilla Public License 2.0
1.29k stars 157 forks source link

Decide (and document) best practices for FFI declarations #554

Open nicholasbishop opened 1 year ago

nicholasbishop commented 1 year ago

We have a lot of FFI declarations in the code, and the style of these declarations is not always consistent:

  1. Most are declared unsafe, but not all.
  2. Some places *const/*mut, others use &/&mut. Using references can be correct since they are layout-compatible with pointers, but it's not always clear from the spec what the restrictions are on how a pointer gets used, so using references may lead to UB.
  3. Some places use wrappers like Option / NonNull. These can be perfectly correct in cases where the layout is well defined and FFI-compatible, but the inconsistency can still make it harder to verify correctness.

Another layer of confusion comes from the IN/OUT/OPTIONAL/CONST modifiers in the spec. Sometimes these are used in weird ways (e.g. OPTIONAL is defined as allowing NULL to be passed in, but sometimes it's applied to non-pointer parameters). Sometimes they seem more like guides than rules (IN and OUT in particular seem more like semantic hints). So in general the safest way to define an FFI pointer is probably to use *mut... but if that pointer is initialized from a & reference in a safe wrapper we provide then that might just be moving the location of possible UB down a level.

See also some good relevant discussion here: https://github.com/r-efi/r-efi/issues/46

timrobertsdev commented 1 year ago

On a fun level, I've found that OPTIONAL doesn't always apply or doesn't apply to the first pointer in a pointer-to-a-pointer parameter as well.

There are more than a few inconsistencies between the spec and EDK2 (and other implementations, I would imagine). I tend to dive into the EDK2 source when something is ambiguous in the spec, but would this be correct? Specifically when it comes to function parameters and their modifiers.