Let's discuss UEFI's pointer conventions about uefi-rs HOT 7 CLOSED

Req. 1: is it even possible to allocate a structure in memory mapped space? I'm pretty sure even regular unsafe Rust code would fail to work in such cases.
I don't think there's anything we can do to guard against this, any caller giving us references into non-physical memory is looking for trouble.

Req. 2 & Req. 3: we can get around this by simply never exposing raw pointers in safe functions. All functions which take non-null pointers use references, and the ones which do allow null pointers take in an Option<&T>. It's the callers responsability for the reference to be valid and aligned.

Req. 4: the way I understand this line, it doesn't talk about the state of the "parameters pointed by a pointer", but rather the pointer parameters.

In other words, if UEFI takes a reference to a Handle, we pass in a &mut Handle, there's no guarantee UEFI will fill the handle with null_ptr on error.

from uefi-rs.

Req. 1: is it even possible to allocate a structure in memory mapped space? I'm pretty sure even regular unsafe Rust code would fail to work in such cases.

As a matter of fact, we do already expose a memory-mapped device: the GOP frame buffer. And indeed, come to think of it, our current testing code probably uses it incorrectly, as volatile writes should be used to avoid optimization problems. Instead of a slice, we should probably expose a slice wrapper which enforces volatile write semantics in order to avoid that kind of issue in the future.

I don't think the spec dictates that firmware data structures cannot be in memory-mapped memory (which could make sense for things like the ACPI tables). That could be another source of hidden memory-mapped IO, but likely a harmless one: since the memory mapping must be transparent to the application, they can't do "dangerous" MMIO stuff like changing data structures under our feet. Therefore, only a very anal UEFI implementation could reject passing that as a parameter to a function (should we ever need to).

Not sure if UEFI exposes any memory-mapped I/O other than the GOP frame buffer and possibly some firmware data structures. I do intend to ultimately check the 2400 pages of spec in order to make uefi-rs achieve full API coverage, but that will take me a while, and right now I do not feel confidence to make a blanket statement about what all UEFI interfaces do and do not. 😄

Req. 2 & Req. 3: we can get around this by simply never exposing raw pointers in safe functions. All functions which take non-null pointers use references, and the ones which do allow null pointers take in an Option<&T>. It's the callers responsability for the reference to be valid and aligned.

As I said, I think we can mostly ignore these requirements since they match normal expectations of unsafe Rust code. Requirements 1 and 4 are the only ones that really get me concerned.

Req. 4: the way I understand this line, it doesn't talk about the state of the "parameters pointed by a pointer", but rather the pointer parameters.

In other words, if UEFI takes a reference to a Handle, we pass in a &mut Handle, there's no guarantee UEFI will fill the handle with null_ptr on error.

I'm not sure if I fully understand the nuance. I suspect that we might be saying the same thing with different words. Can you provide an example of a situation where our two understandings of this requirement differ?

My understanding is that if a UEFI function that takes an *mut T as an input fails, the data of type T behind that pointer should be considered corrupted, and any safety invariants of type T would have to be manually restored by a safe interface.

So for example, if we send in an *mut NotNull<T> to a faillible UEFI function that is exposed via a safe interface, then we should backup the original value of the NotNull pointer before calling UEFI and restore it if the UEFI function fails (since for all we know UEFI could have replaced it with a null pointer or something equally invalid).

from uefi-rs.

our current testing code probably uses it incorrectly, as volatile writes should be used to avoid optimization problems

It's debatable what the correct usage is, since the spec doesn't mention if this memory is write-combined or something.

if we send in an *mut NotNull to a faillible UEFI function that is exposed via a safe interface

Well, my point is that we should not use pointers in the safe API.

For example, look at the memory_map function: it is perfectly safe. It takes in a reference to a mutable buffer. We never specify what happens to that memory.

• If the function fails, then the buffer can get corrupted, but it's OK, since it's just an array of u8s.
• If the function succeeds, then we know the buffer contains valid data, and return an iterator which returns references to the memory descriptors.

At no point do we take a mutable reference to a T and risk corrupting it. All the functions in the crate take in parameters by value or by const reference (so we cannot fill them with invalid data), and we only return them when the function succeeds.

from uefi-rs.

It's debatable what the correct usage is, since the spec doesn't mention if this memory is write-combined or something.

It's true that the proper usage pattern on the hardware side is unclear. However, the current usage is provably incorrect at the language level. The Rust compiler is within its right to optimize out any non-volatile write to memory, especially if it does not get subsequently read by the program, since it works under the assumption that the program is the only entity that will ever read from memory. This is what I would like to address by providing a higher-level abstraction on top of the GOP framebuffer which enforces volatile writes.

I agree with you that the memory_map function has a very good way to deal with UEFI's limited data consistency guarantees. Here, I'm looking into ways to hint faillible uefi-rs developers like myself into remembering that this problem exists and must be dealt with (using designs like the one from memory_map) whenever a new API function is being interfaced.

from uefi-rs.

These requirements cannot be cleanly encoded in the type system without something akin to the efiapi macro discussed in #41, which is itself unfeasible within Rust's current macro system. I'll try to document this kind of considerations in the contribution guide instead.

from uefi-rs.

@HadrienG2 We should try to make this lib as safe-ish as possible, but not any safer :)

Considering the huge amount of ways low-level developers can shoot themselves in the foot, it's doubtful we can truly provide memory safety in all cases without limiting the usage of the UEFI API.
Right now the API is based on a "don't do anything dubious and nothing bad should happen" principle.

If people find uefi-rs succesful in practice, then that's good enough for me.

from uefi-rs.

I'm trying to reach the level of safety that is normally guaranteed by Rust: APIs should be safe to use as long as...

1. User triggers no undefined behavior
2. The safety contracts of unsafe functions are upheld
3. There is no bug in the implementation

This means that some UEFI APIs cannot be exposed in a safe form. But that's okay, we can just expose them as unsafe APIs with well-documented contracts :)

from uefi-rs.