Re: [PATCH v3 1/4] rust: uaccess: add userspace pointers

[Benno Lossin]

On 3/11/24 11:47, Alice Ryhl wrote:
> From: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
> 
> A pointer to an area in userspace memory, which can be either read-only
> or read-write.
> 
> All methods on this struct are safe: invalid pointers return `EFAULT`.
> Concurrent access, *including data races to/from userspace memory*, is
> permitted, because fundamentally another userspace thread/process could
> always be modifying memory at the same time (in the same way that
> userspace Rust's `std::io` permits data races with the contents of
> files on disk). In the presence of a race, the exact byte values
> read/written are unspecified but the operation is well-defined.
> Kernelspace code should validate its copy of data after completing a
> read, and not expect that multiple reads of the same address will return
> the same value.
> 
> These APIs are designed to make it difficult to accidentally write
> TOCTOU bugs. Every time you read from a memory location, the pointer is
> advanced by the length so that you cannot use that reader to read the
> same memory location twice. Preventing double-fetches avoids TOCTOU
> bugs. This is accomplished by taking `self` by value to prevent
> obtaining multiple readers on a given `UserSlicePtr`, and the readers
> only permitting forward reads. If double-fetching a memory location is
> necessary for some reason, then that is done by creating multiple
> readers to the same memory location.
> 
> Constructing a `UserSlicePtr` performs no checks on the provided
> address and length, it can safely be constructed inside a kernel thread
> with no current userspace process. Reads and writes wrap the kernel APIs
> `copy_from_user` and `copy_to_user`, which check the memory map of the
> current process and enforce that the address range is within the user
> range (no additional calls to `access_ok` are needed).
> 
> This code is based on something that was originally written by Wedson on
> the old rust branch. It was modified by Alice by removing the
> `IoBufferReader` and `IoBufferWriter` traits, and various other changes.
> 
> Signed-off-by: Wedson Almeida Filho <wedsonaf@xxxxxxxxx>
> Co-developed-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>

Reviewed-by: Benno Lossin <benno.lossin@xxxxxxxxx>