RFC: "Raw" pointer improvements

[jstarks]

LiteBox has evolved such that "raw" pointers essentially mean "guest"/"user" pointers--they represent untrusted addresses under user control, outside of the Rust memory model. Although one can imagine uses of LiteBox where the pointers can be trusted, no one writing generic shim code can assume this.

As such, there are several challenges with the current design:

Safety

Many of the pointer trait methods are unsafe, but the safety contract is unclear and/or unenforceable. These pointers are inherently untrusted--they come from code on the other side of the security barrier (the Linux process, the OPTEE TA).

And there is no mechanism that the shim could use to validate a address ahead of time, before constructing a pointer, since in most cases this would be a TOCTOU bug--the user process can change the page mappings between pointer validation and pointer use. The best we can do is validate that the address is outside of the kernel address range... but we might as well defer that to access time if we're going to perform the check at all (on some architectures, it can be combined with the access).

We should fix this so that all trait methods on raw pointers are safe, relying on the platform to ensure this. For userland platforms, where this is not practical and there is no security boundary, this will have to be best-effort.

COW

The pointer trait methods provide ways to get borrowed Cow references to the user data but, as discussed above, this is always unsafe and undefined behavior--guest memory lives outside the Rust memory model. We should eliminate this altogether.

Safe transmutability

Currently, the pointer trait access methods do not require that the underlying data can be safely transmuted to/from bytes--it's up to the caller to not read an Arc<u32> or &'static Platform from guest memory. We can leverage the zerocopy crate to provide useful bounds while still allowing safe access of user types.

Atomic accesses

Some code (currently, just the futex manager) needs a guarantee that a given memory access is performed atomically. There is no mechanism for this today, and so code that has this requirement currently transmutes the address to an &AtomicU32 and hopes for the best.

Ergonomics

The current pointers are difficult to use correctly for some use cases.

• There are multiple overlapping methods to read/write data, but no simple read/write methods for the common case where you just want to access the pointed-to data.
• It is difficult to cast a pointer between types or to construct a pointer that is offset from another one. You can cast through usize, but the traits warn you not to do this.
• Accessing a slice of guest memory requires extra care since you are responsible for all bounds checks. There's no mechanism to create a "raw slice".
• Because LiteBox code uses the platform pointer objects directly, it can be awkward to describe the pointer type in generic code. See uses of IoReadVec for an example.

Proposal

To address all these, we should split the guest pointer infrastructure into two parts:

1. A simple RawPointer trait implemented by the platform to provide low-level memory access in a safe way.
2. Rich XxxPointer<T> types provided by litebox that the shims use to actually access memory.

The low-level trait would look something like:

trait RawPointer {
    // Safe if `T` is safe to read. Guaranteed to be atomic if `T` is 1,2,4,8 byes and aligned.
    unsafe fn read<T>(self) -> Result<T, Fault>;
    // Safe if `T` is safe to write.
    unsafe fn write_ref<T>(self, v: &T) -> Result<(), Fault>;
    // Always safe.
    fn read_bytes(self, dst: &mut [u8]) -> Result<(), Fault>;
    fn write_bytes(self, src: &[u8]) -> Result<(), Fault>;
    fn from_addr(_: usize) -> Self;
    fn addr(self) -> usize;
    fn offset(self, offset: isize) -> Self;
}

And the high-level objects would look like:

struct MutPointer<Platform, T:?Sized>(Platform::RawPointer, PhantomData<*mut T>);

impl<Platform, T> MutPointer<T> {
    fn read(self) -> Result<T, Fault> where T: zerocopy::FromBytes { ... }
    fn write(self, v: T) -> Result<(), Fault> where T: zerocopy::IntoBytes {...}
    fn from_addr(_: usize) -> Self {...}   
    fn offset(self, count: isize) -> Self {...}
}

impl<Platform, T> MutPointer<[T]> {
    fn from_addr_and_len(addr: usize, len: usize) -> Self {...}
    fn copy_from_slice(self, data: &[T]) -> Result<(), Fault> where T: zerocopy::FromBytes {...}
    fn copy_into_slice(self, buf: &mut [T]) -> Result<(), Fault> where T: zerocopy::IntoBytes {...}
    fn len(self) -> usize;
    fn get<I>(self, range: I) -> Option<Self> {...}
}

impl<Platform, T:?Sized> MutPointer<T> {
    fn cast_const(self) -> ConstPointer<T> {...}
    fn cast<U>(self) -> MutPointer<U> {...}
    fn addr(self) -> usize {...}
}

This approach clearly delineates the responsibilities between the platform and LiteBox:

• The platform is responsible for ensuring the correctness, safety, and performance of the raw operations, e.g., ensuring that the memory accesses are really to guest memory, that aligned accesses are atomic, etc.
• LiteBox is responsible for ensuring Rust-level safety (adding in the zerocopy bounds, for example) and ergonomics (generic for avoiding pointer confusion, support for slice types, etc.).

[jaybosamiya-ms]

Thanks John for kicking off a conversation about the raw pointer bits. I have quite a few thoughts (and also some local changes) for a better design for the raw pointer APIs, but haven't had a chance to push them all out yet.

There are many good points you bring up above, but just making a quick note that the reason for the Cow was for purely-userland scenarios to remove the effectively double memcpy. We haven't taken full advantage of it, but yeah that's the reason for it (and also the reason I had originally decided not to call things guest/host pointers).

As a quick summary, I like the ideas you've written, and they largely are along the direction I had in mind. Will write a more detailed response later.

Regarding ergonomics + design issues things are kinda scattered across a collection of issues. Just listing a bunch of related issues here as stuff to keep in mind when setting up the new design:

• Where should we check untrusted pointers into memory? #6
• Checking validity of user-space pointers in kernel land #283
• Improving atomic user pointer support #296
• Adding new interface to RawMutPointer to allow to write one field of a struct #396
• Remove all the transmutes #145
• Consider Send/Sync-ness of RawConstPtr and friends #431

[jstarks]

There are many good points you bring up above, but just making a quick note that the reason for the Cow was for purely-userland scenarios to remove the effectively double memcpy. We haven't taken full advantage of it, but yeah that's the reason for it (and also the reason I had originally decided not to call things guest/host pointers).

Yeah, I figured. I think this is an interesting idea but ultimately is probably the wrong tradeoff. I don't think you could convince me that it can be made sound, even for the userland platforms, since syscall arguments are not really required to be immutable for the lifetime of the syscall... but Cow::Borrowed certainly requires that.

Plus, there are not many places where these user/kernel memory copies matter for performance. Those places that do exist can probably be fixed in other ways. E.g., let's pass user pointers deeper into the file system, not rely on the shim to copy to/from "kernel" memory. Most of the file systems today can operate on user memory just as easily as kernel memory.

For that to work while still allowing you to pass a kernel pointer if you want, we'll probably need something like:

enum MaybeUserPointer<'a, T> {
    User(UserPointer<T>),
    Kernel(&'a T),
}

And then we can implement all the same methods on this enum as we will on UserPointer.

[jaybosamiya-ms]

Just making a quick note that when moving to the nicer version of "raw" pointers, we might want to keep the ptr.rs thoughts (for physical memory and such) from #555 in mind.