diff mbox series

[v6,4/4] rust: add abstraction for `struct page`

Message ID 20240418-alice-mm-v6-4-cb8f3e5d688f@google.com (mailing list archive)
State New
Headers show
Series Memory management patches needed by Rust Binder | expand

Commit Message

Alice Ryhl April 18, 2024, 8:59 a.m. UTC
Adds a new struct called `Page` that wraps a pointer to `struct page`.
This struct is assumed to hold ownership over the page, so that Rust
code can allocate and manage pages directly.

The page type has various methods for reading and writing into the page.
These methods will temporarily map the page to allow the operation. All
of these methods use a helper that takes an offset and length, performs
bounds checks, and returns a pointer to the given offset in the page.

This patch only adds support for pages of order zero, as that is all
Rust Binder needs. However, it is written to make it easy to add support
for higher-order pages in the future. To do that, you would add a const
generic parameter to `Page` that specifies the order. Most of the
methods do not need to be adjusted, as the logic for dealing with
mapping multiple pages at once can be isolated to just the
`with_pointer_into_page` method.

Rust Binder needs to manage pages directly as that is how transactions
are delivered: Each process has an mmap'd region for incoming
transactions. When an incoming transaction arrives, the Binder driver
will choose a region in the mmap, allocate and map the relevant pages
manually, and copy the incoming transaction directly into the page. This
architecture allows the driver to copy transactions directly from the
address space of one process to another, without an intermediate copy
to a kernel buffer.

This code is based on Wedson's page abstractions from the old rust
branch, but it has been modified by Alice by removing the incomplete
support for higher-order pages, by introducing the `with_*` helpers
to consolidate the bounds checking logic into a single place, and
various other changes.

Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>
Reviewed-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers.c                  |  20 ++++
 rust/kernel/alloc.rs            |   7 ++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/page.rs             | 250 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 279 insertions(+)

Comments

Boqun Feng April 18, 2024, 6:52 p.m. UTC | #1
On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
> Adds a new struct called `Page` that wraps a pointer to `struct page`.
> This struct is assumed to hold ownership over the page, so that Rust
> code can allocate and manage pages directly.
> 
> The page type has various methods for reading and writing into the page.
> These methods will temporarily map the page to allow the operation. All
> of these methods use a helper that takes an offset and length, performs
> bounds checks, and returns a pointer to the given offset in the page.
> 
> This patch only adds support for pages of order zero, as that is all
> Rust Binder needs. However, it is written to make it easy to add support
> for higher-order pages in the future. To do that, you would add a const
> generic parameter to `Page` that specifies the order. Most of the
> methods do not need to be adjusted, as the logic for dealing with
> mapping multiple pages at once can be isolated to just the
> `with_pointer_into_page` method.
> 

Thank you for doing this, and breaking the chicken-and-egg problem chain
;-) For sure, the whole package of page API would need more time to
design, implement and review, but this patch looks good enough to me.

> Rust Binder needs to manage pages directly as that is how transactions
> are delivered: Each process has an mmap'd region for incoming
> transactions. When an incoming transaction arrives, the Binder driver
> will choose a region in the mmap, allocate and map the relevant pages
> manually, and copy the incoming transaction directly into the page. This
> architecture allows the driver to copy transactions directly from the
> address space of one process to another, without an intermediate copy
> to a kernel buffer.
> 
> This code is based on Wedson's page abstractions from the old rust
> branch, but it has been modified by Alice by removing the incomplete
> support for higher-order pages, by introducing the `with_*` helpers
> to consolidate the bounds checking logic into a single place, and
> various other changes.
> 
> Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>
> Reviewed-by: Trevor Gross <tmgross@umich.edu>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Something I want to bring up for discussion below:

[...]

> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
> +    ///
> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
> +    /// this task, as this method uses a local mapping.
> +    ///
> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
> +    /// `EINVAL` and does not call `f`.
> +    ///
> +    /// # Using the raw pointer
> +    ///
> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
> +    /// data races, the memory may be uninitialized, and so on.
> +    ///
> +    /// If multiple threads map the same page at the same time, then they may reference with
> +    /// different addresses. However, even if the addresses are different, the underlying memory is
> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
> +    /// same underlying byte at the same time).
> +    fn with_pointer_into_page<T>(
> +        &self,
> +        off: usize,
> +        len: usize,
> +        f: impl FnOnce(*mut u8) -> Result<T>,

I wonder whether the way to go here is making this function signature:

    fn with_slice_in_page<T> (
        &self,
	off: usize,
	len: usize,
	f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
    ) -> Result<T>

, because in this way, it makes a bit more clear that what memory that
`f` can access, in other words, the users are less likely to use the
pointer in a wrong way.

But that depends on whether `&UnsafeCell<[u8]>` is the correct
abstraction and the ecosystem around it: for example, I feel like these
two functions:

	fn len(slice: &UnsafeCell<[u8]>) -> usize
	fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8

should be trivially safe, but I might be wrong. Again this is just for
future discussion.

Regards,
Boqun

> +    ) -> Result<T> {
> +        let bounds_ok = off <= PAGE_SIZE && len <= PAGE_SIZE && (off + len) <= PAGE_SIZE;
> +
> +        if bounds_ok {
> +            self.with_page_mapped(move |page_addr| {
> +                // SAFETY: The `off` integer is at most `PAGE_SIZE`, so this pointer offset will
> +                // result in a pointer that is in bounds or one off the end of the page.
> +                f(unsafe { page_addr.add(off) })
> +            })
> +        } else {
> +            Err(EINVAL)
> +        }
> +    }
> +
[...]
> 
> -- 
> 2.44.0.683.g7961c838ac-goog
>
Benno Lossin April 18, 2024, 10:08 p.m. UTC | #2
On 18.04.24 20:52, Boqun Feng wrote:
> On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
>> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
>> +    ///
>> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
>> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
>> +    /// this task, as this method uses a local mapping.
>> +    ///
>> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
>> +    /// `EINVAL` and does not call `f`.
>> +    ///
>> +    /// # Using the raw pointer
>> +    ///
>> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
>> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
>> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
>> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
>> +    /// data races, the memory may be uninitialized, and so on.
>> +    ///
>> +    /// If multiple threads map the same page at the same time, then they may reference with
>> +    /// different addresses. However, even if the addresses are different, the underlying memory is
>> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
>> +    /// same underlying byte at the same time).
>> +    fn with_pointer_into_page<T>(
>> +        &self,
>> +        off: usize,
>> +        len: usize,
>> +        f: impl FnOnce(*mut u8) -> Result<T>,
> 
> I wonder whether the way to go here is making this function signature:
> 
>      fn with_slice_in_page<T> (
>          &self,
> 	       off: usize,
> 	       len: usize,
> 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
>      ) -> Result<T>
> 
> , because in this way, it makes a bit more clear that what memory that
> `f` can access, in other words, the users are less likely to use the
> pointer in a wrong way.
> 
> But that depends on whether `&UnsafeCell<[u8]>` is the correct
> abstraction and the ecosystem around it: for example, I feel like these
> two functions:
> 
> 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
> 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
> 
> should be trivially safe, but I might be wrong. Again this is just for
> future discussion.

I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
can always access the length.

Another question would be if page allows for uninitialized bits, in that
case, we would need `&[Opaque<u8>]`.

But I don't remember how to get a valid raw pointer from
`&[UnsafeCell<u8>]`.
Boqun Feng April 18, 2024, 10:56 p.m. UTC | #3
On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
> On 18.04.24 20:52, Boqun Feng wrote:
> > On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
> >> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
> >> +    ///
> >> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
> >> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
> >> +    /// this task, as this method uses a local mapping.
> >> +    ///
> >> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
> >> +    /// `EINVAL` and does not call `f`.
> >> +    ///
> >> +    /// # Using the raw pointer
> >> +    ///
> >> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
> >> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
> >> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
> >> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
> >> +    /// data races, the memory may be uninitialized, and so on.
> >> +    ///
> >> +    /// If multiple threads map the same page at the same time, then they may reference with
> >> +    /// different addresses. However, even if the addresses are different, the underlying memory is
> >> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
> >> +    /// same underlying byte at the same time).
> >> +    fn with_pointer_into_page<T>(
> >> +        &self,
> >> +        off: usize,
> >> +        len: usize,
> >> +        f: impl FnOnce(*mut u8) -> Result<T>,
> > 
> > I wonder whether the way to go here is making this function signature:
> > 
> >      fn with_slice_in_page<T> (
> >          &self,
> > 	       off: usize,
> > 	       len: usize,
> > 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
> >      ) -> Result<T>
> > 
> > , because in this way, it makes a bit more clear that what memory that
> > `f` can access, in other words, the users are less likely to use the
> > pointer in a wrong way.
> > 
> > But that depends on whether `&UnsafeCell<[u8]>` is the correct
> > abstraction and the ecosystem around it: for example, I feel like these
> > two functions:
> > 
> > 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
> > 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
> > 
> > should be trivially safe, but I might be wrong. Again this is just for
> > future discussion.
> 
> I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
> can always access the length.
> 

Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
[u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
right? I haven't found any method doing that, but the length should be
just a part of fat pointer, so I think getting that is a defined
behavior. But maybe I'm missing something.

> Another question would be if page allows for uninitialized bits, in that
> case, we would need `&[Opaque<u8>]`.
> 

Yes, or `&Opaque<[u8>]`.

Regards,
Boqun

> But I don't remember how to get a valid raw pointer from
> `&[UnsafeCell<u8>]`.
> 
> -- 
> Cheers,
> Benno
>
Boqun Feng April 18, 2024, 11:04 p.m. UTC | #4
On Thu, Apr 18, 2024 at 03:56:11PM -0700, Boqun Feng wrote:
> On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
> > On 18.04.24 20:52, Boqun Feng wrote:
> > > On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
> > >> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
> > >> +    ///
> > >> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
> > >> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
> > >> +    /// this task, as this method uses a local mapping.
> > >> +    ///
> > >> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
> > >> +    /// `EINVAL` and does not call `f`.
> > >> +    ///
> > >> +    /// # Using the raw pointer
> > >> +    ///
> > >> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
> > >> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
> > >> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
> > >> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
> > >> +    /// data races, the memory may be uninitialized, and so on.
> > >> +    ///
> > >> +    /// If multiple threads map the same page at the same time, then they may reference with
> > >> +    /// different addresses. However, even if the addresses are different, the underlying memory is
> > >> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
> > >> +    /// same underlying byte at the same time).
> > >> +    fn with_pointer_into_page<T>(
> > >> +        &self,
> > >> +        off: usize,
> > >> +        len: usize,
> > >> +        f: impl FnOnce(*mut u8) -> Result<T>,
> > > 
> > > I wonder whether the way to go here is making this function signature:
> > > 
> > >      fn with_slice_in_page<T> (
> > >          &self,
> > > 	       off: usize,
> > > 	       len: usize,
> > > 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
> > >      ) -> Result<T>
> > > 
> > > , because in this way, it makes a bit more clear that what memory that
> > > `f` can access, in other words, the users are less likely to use the
> > > pointer in a wrong way.
> > > 
> > > But that depends on whether `&UnsafeCell<[u8]>` is the correct
> > > abstraction and the ecosystem around it: for example, I feel like these
> > > two functions:
> > > 
> > > 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
> > > 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
> > > 
> > > should be trivially safe, but I might be wrong. Again this is just for
> > > future discussion.
> > 
> > I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
> > can always access the length.
> > 
> 
> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
> right? I haven't found any method doing that, but the length should be
> just a part of fat pointer, so I think getting that is a defined
> behavior. But maybe I'm missing something.
> 

Hmm... but I guess one of the problems of this approach, is how to
construct a `&UnsafeCell<[u8]>` from a pointer and length...

Regards,
Boqun

> > Another question would be if page allows for uninitialized bits, in that
> > case, we would need `&[Opaque<u8>]`.
> > 
> 
> Yes, or `&Opaque<[u8>]`.
> 
> Regards,
> Boqun
> 
> > But I don't remember how to get a valid raw pointer from
> > `&[UnsafeCell<u8>]`.
> > 
> > -- 
> > Cheers,
> > Benno
> >
Benno Lossin April 19, 2024, 8:36 a.m. UTC | #5
On 19.04.24 01:04, Boqun Feng wrote:
> On Thu, Apr 18, 2024 at 03:56:11PM -0700, Boqun Feng wrote:
>> On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
>>> On 18.04.24 20:52, Boqun Feng wrote:
>>>> On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
>>>>> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
>>>>> +    ///
>>>>> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
>>>>> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
>>>>> +    /// this task, as this method uses a local mapping.
>>>>> +    ///
>>>>> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
>>>>> +    /// `EINVAL` and does not call `f`.
>>>>> +    ///
>>>>> +    /// # Using the raw pointer
>>>>> +    ///
>>>>> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
>>>>> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
>>>>> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
>>>>> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
>>>>> +    /// data races, the memory may be uninitialized, and so on.
>>>>> +    ///
>>>>> +    /// If multiple threads map the same page at the same time, then they may reference with
>>>>> +    /// different addresses. However, even if the addresses are different, the underlying memory is
>>>>> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
>>>>> +    /// same underlying byte at the same time).
>>>>> +    fn with_pointer_into_page<T>(
>>>>> +        &self,
>>>>> +        off: usize,
>>>>> +        len: usize,
>>>>> +        f: impl FnOnce(*mut u8) -> Result<T>,
>>>>
>>>> I wonder whether the way to go here is making this function signature:
>>>>
>>>>       fn with_slice_in_page<T> (
>>>>           &self,
>>>> 	       off: usize,
>>>> 	       len: usize,
>>>> 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
>>>>       ) -> Result<T>
>>>>
>>>> , because in this way, it makes a bit more clear that what memory that
>>>> `f` can access, in other words, the users are less likely to use the
>>>> pointer in a wrong way.
>>>>
>>>> But that depends on whether `&UnsafeCell<[u8]>` is the correct
>>>> abstraction and the ecosystem around it: for example, I feel like these
>>>> two functions:
>>>>
>>>> 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
>>>> 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
>>>>
>>>> should be trivially safe, but I might be wrong. Again this is just for
>>>> future discussion.
>>>
>>> I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
>>> can always access the length.
>>>
>>
>> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
>> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
>> right? I haven't found any method doing that, but the length should be
>> just a part of fat pointer, so I think getting that is a defined
>> behavior. But maybe I'm missing something.

There is `to_raw_parts` [1], but that is unstable. (Note that
`<[T] as Pointee>::Metadata = usize`, see [2])

>>
> 
> Hmm... but I guess one of the problems of this approach, is how to
> construct a `&UnsafeCell<[u8]>` from a pointer and length...

We could use `from_raw_parts` [3]. But when making the slice the outer
type, we can use a stable function to convert a pointer and a length to
a slice [4].

> 
> Regards,
> Boqun
> 
>>> Another question would be if page allows for uninitialized bits, in that
>>> case, we would need `&[Opaque<u8>]`.
>>>
>>
>> Yes, or `&Opaque<[u8>]`.

I don't think that putting the slice on the inside is what we want. Also
note that `Opaque<T>` requires that `T: Sized` and that is not the case
for `[u8]`.

[1]: https://doc.rust-lang.org/nightly/core/primitive.pointer.html#method.to_raw_parts
[2]: https://doc.rust-lang.org/nightly/core/ptr/trait.Pointee.html#pointer-metadata
[3]: https://doc.rust-lang.org/nightly/core/ptr/fn.from_raw_parts.html
[4]: https://doc.rust-lang.org/nightly/core/slice/fn.from_raw_parts.html
Boqun Feng April 19, 2024, 5:23 p.m. UTC | #6
On Fri, Apr 19, 2024 at 08:36:11AM +0000, Benno Lossin wrote:
> On 19.04.24 01:04, Boqun Feng wrote:
> > On Thu, Apr 18, 2024 at 03:56:11PM -0700, Boqun Feng wrote:
> >> On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
> >>> On 18.04.24 20:52, Boqun Feng wrote:
> >>>> On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
> >>>>> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
> >>>>> +    ///
> >>>>> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
> >>>>> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
> >>>>> +    /// this task, as this method uses a local mapping.
> >>>>> +    ///
> >>>>> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
> >>>>> +    /// `EINVAL` and does not call `f`.
> >>>>> +    ///
> >>>>> +    /// # Using the raw pointer
> >>>>> +    ///
> >>>>> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
> >>>>> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
> >>>>> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
> >>>>> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
> >>>>> +    /// data races, the memory may be uninitialized, and so on.
> >>>>> +    ///
> >>>>> +    /// If multiple threads map the same page at the same time, then they may reference with
> >>>>> +    /// different addresses. However, even if the addresses are different, the underlying memory is
> >>>>> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
> >>>>> +    /// same underlying byte at the same time).
> >>>>> +    fn with_pointer_into_page<T>(
> >>>>> +        &self,
> >>>>> +        off: usize,
> >>>>> +        len: usize,
> >>>>> +        f: impl FnOnce(*mut u8) -> Result<T>,
> >>>>
> >>>> I wonder whether the way to go here is making this function signature:
> >>>>
> >>>>       fn with_slice_in_page<T> (
> >>>>           &self,
> >>>> 	       off: usize,
> >>>> 	       len: usize,
> >>>> 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
> >>>>       ) -> Result<T>
> >>>>
> >>>> , because in this way, it makes a bit more clear that what memory that
> >>>> `f` can access, in other words, the users are less likely to use the
> >>>> pointer in a wrong way.
> >>>>
> >>>> But that depends on whether `&UnsafeCell<[u8]>` is the correct
> >>>> abstraction and the ecosystem around it: for example, I feel like these
> >>>> two functions:
> >>>>
> >>>> 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
> >>>> 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
> >>>>
> >>>> should be trivially safe, but I might be wrong. Again this is just for
> >>>> future discussion.
> >>>
> >>> I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
> >>> can always access the length.
> >>>
> >>
> >> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
> >> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
> >> right? I haven't found any method doing that, but the length should be
> >> just a part of fat pointer, so I think getting that is a defined
> >> behavior. But maybe I'm missing something.
> 
> There is `to_raw_parts` [1], but that is unstable. (Note that
> `<[T] as Pointee>::Metadata = usize`, see [2])
> 

Oh, that's good to know, thank you! ;-)

> >>
> > 
> > Hmm... but I guess one of the problems of this approach, is how to
> > construct a `&UnsafeCell<[u8]>` from a pointer and length...
> 
> We could use `from_raw_parts` [3]. But when making the slice the outer
> type, we can use a stable function to convert a pointer and a length to
> a slice [4].
> 

Yes, but there appears no way to get a pointer with larger provenance
from a `&[UnsafeCell<u8>]`, right?

> > 
> > Regards,
> > Boqun
> > 
> >>> Another question would be if page allows for uninitialized bits, in that
> >>> case, we would need `&[Opaque<u8>]`.
> >>>
> >>
> >> Yes, or `&Opaque<[u8>]`.
> 
> I don't think that putting the slice on the inside is what we want. Also

Hmm.. why? So in `&UnsafeCell<[u8]>` vs `&[UnsafeCell<u8>]` case, I
think the former represent "a slice of u8 that can be modified in the
same time" very well, and this is what a pointer-and-length pair usually
represents in kernel, I think. But yes, the latter is OK to me as well,
just hard to play the provenance game I guess?

> note that `Opaque<T>` requires that `T: Sized` and that is not the case
> for `[u8]`.

Oh, you're right. In case of MaybeUninit, it requires `T: Sized`, so
`Opaque<[u8]>` doesn't quite work.

Moving forward, maybe the first step is to see whether `&[Opaque<u8>]`
and `&[UnsafeCell<u8>]` can have a good way to generate a pointer with
proper provenance? Time to ping t-opsem maybe?

Regards,
Boqun

> 
> [1]: https://doc.rust-lang.org/nightly/core/primitive.pointer.html#method.to_raw_parts
> [2]: https://doc.rust-lang.org/nightly/core/ptr/trait.Pointee.html#pointer-metadata
> [3]: https://doc.rust-lang.org/nightly/core/ptr/fn.from_raw_parts.html
> [4]: https://doc.rust-lang.org/nightly/core/slice/fn.from_raw_parts.html
> 
> -- 
> Cheers,
> Benno
>
Benno Lossin April 19, 2024, 7:24 p.m. UTC | #7
On 19.04.24 19:23, Boqun Feng wrote:
> On Fri, Apr 19, 2024 at 08:36:11AM +0000, Benno Lossin wrote:
>> On 19.04.24 01:04, Boqun Feng wrote:
>>> On Thu, Apr 18, 2024 at 03:56:11PM -0700, Boqun Feng wrote:
>>>> On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
>>>>> On 18.04.24 20:52, Boqun Feng wrote:
>>>>>> On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
>>>>>>> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
>>>>>>> +    ///
>>>>>>> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
>>>>>>> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
>>>>>>> +    /// this task, as this method uses a local mapping.
>>>>>>> +    ///
>>>>>>> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
>>>>>>> +    /// `EINVAL` and does not call `f`.
>>>>>>> +    ///
>>>>>>> +    /// # Using the raw pointer
>>>>>>> +    ///
>>>>>>> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
>>>>>>> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
>>>>>>> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
>>>>>>> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
>>>>>>> +    /// data races, the memory may be uninitialized, and so on.
>>>>>>> +    ///
>>>>>>> +    /// If multiple threads map the same page at the same time, then they may reference with
>>>>>>> +    /// different addresses. However, even if the addresses are different, the underlying memory is
>>>>>>> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
>>>>>>> +    /// same underlying byte at the same time).
>>>>>>> +    fn with_pointer_into_page<T>(
>>>>>>> +        &self,
>>>>>>> +        off: usize,
>>>>>>> +        len: usize,
>>>>>>> +        f: impl FnOnce(*mut u8) -> Result<T>,
>>>>>>
>>>>>> I wonder whether the way to go here is making this function signature:
>>>>>>
>>>>>>        fn with_slice_in_page<T> (
>>>>>>            &self,
>>>>>> 	       off: usize,
>>>>>> 	       len: usize,
>>>>>> 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
>>>>>>        ) -> Result<T>
>>>>>>
>>>>>> , because in this way, it makes a bit more clear that what memory that
>>>>>> `f` can access, in other words, the users are less likely to use the
>>>>>> pointer in a wrong way.
>>>>>>
>>>>>> But that depends on whether `&UnsafeCell<[u8]>` is the correct
>>>>>> abstraction and the ecosystem around it: for example, I feel like these
>>>>>> two functions:
>>>>>>
>>>>>> 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
>>>>>> 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
>>>>>>
>>>>>> should be trivially safe, but I might be wrong. Again this is just for
>>>>>> future discussion.
>>>>>
>>>>> I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
>>>>> can always access the length.
>>>>>
>>>>
>>>> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
>>>> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
>>>> right? I haven't found any method doing that, but the length should be
>>>> just a part of fat pointer, so I think getting that is a defined
>>>> behavior. But maybe I'm missing something.
>>
>> There is `to_raw_parts` [1], but that is unstable. (Note that
>> `<[T] as Pointee>::Metadata = usize`, see [2])
>>
> 
> Oh, that's good to know, thank you! ;-)
>
>>> Hmm... but I guess one of the problems of this approach, is how to
>>> construct a `&UnsafeCell<[u8]>` from a pointer and length...
>>
>> We could use `from_raw_parts` [3]. But when making the slice the outer
>> type, we can use a stable function to convert a pointer and a length to
>> a slice [4].
>>
> 
> Yes, but there appears no way to get a pointer with larger provenance
> from a `&[UnsafeCell<u8>]`, right?

What do you mean by "larger provenance"?

>>>>> Another question would be if page allows for uninitialized bits, in that
>>>>> case, we would need `&[Opaque<u8>]`.
>>>>>
>>>>
>>>> Yes, or `&Opaque<[u8>]`.
>>
>> I don't think that putting the slice on the inside is what we want. Also
> 
> Hmm.. why? So in `&UnsafeCell<[u8]>` vs `&[UnsafeCell<u8>]` case, I
> think the former represent "a slice of u8 that can be modified in the
> same time" very well, and this is what a pointer-and-length pair usually
> represents in kernel, I think. But yes, the latter is OK to me as well,
> just hard to play the provenance game I guess?

Ultimately it again comes down to missing field projections :)

The type `&UnsafeCell<[u8]>` is less *useful*, since you cannot even get
the length of the slice. Also indexing into this type is not easily
possible. This is because the only way to get/change the inner value of
an `UnsafeCell` is via `get`.

Compare this with the slice type. It allows getting the length, indexing
into it (ie a form of field projections, if we consider slices as having
a variable amount of fields).

All those issues would be solved by (good) field projections.


Field projections also give a reason for why using `&[UnsafeCell<u8>]`
is not really different from `&UnsafeCell<[u8]>`: At any point in time
we ought to be able to project `&UnsafeCell<[u8]> -> &[UnsafeCell<u8>]`.

So it's fine to just use that from the get-go.

>> note that `Opaque<T>` requires that `T: Sized` and that is not the case
>> for `[u8]`.
> 
> Oh, you're right. In case of MaybeUninit, it requires `T: Sized`, so
> `Opaque<[u8]>` doesn't quite work.
> 
> Moving forward, maybe the first step is to see whether `&[Opaque<u8>]`
> and `&[UnsafeCell<u8>]` can have a good way to generate a pointer with
> proper provenance? Time to ping t-opsem maybe?

Good idea, do you want to do that, or should I do it?
Boqun Feng April 19, 2024, 7:35 p.m. UTC | #8
On Fri, Apr 19, 2024 at 07:24:31PM +0000, Benno Lossin wrote:
> On 19.04.24 19:23, Boqun Feng wrote:
> > On Fri, Apr 19, 2024 at 08:36:11AM +0000, Benno Lossin wrote:
> >> On 19.04.24 01:04, Boqun Feng wrote:
> >>> On Thu, Apr 18, 2024 at 03:56:11PM -0700, Boqun Feng wrote:
> >>>> On Thu, Apr 18, 2024 at 10:08:40PM +0000, Benno Lossin wrote:
> >>>>> On 18.04.24 20:52, Boqun Feng wrote:
> >>>>>> On Thu, Apr 18, 2024 at 08:59:20AM +0000, Alice Ryhl wrote:
> >>>>>>> +    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
> >>>>>>> +    ///
> >>>>>>> +    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
> >>>>>>> +    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
> >>>>>>> +    /// this task, as this method uses a local mapping.
> >>>>>>> +    ///
> >>>>>>> +    /// If `off` and `len` refers to a region outside of this page, then this method returns
> >>>>>>> +    /// `EINVAL` and does not call `f`.
> >>>>>>> +    ///
> >>>>>>> +    /// # Using the raw pointer
> >>>>>>> +    ///
> >>>>>>> +    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
> >>>>>>> +    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
> >>>>>>> +    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
> >>>>>>> +    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
> >>>>>>> +    /// data races, the memory may be uninitialized, and so on.
> >>>>>>> +    ///
> >>>>>>> +    /// If multiple threads map the same page at the same time, then they may reference with
> >>>>>>> +    /// different addresses. However, even if the addresses are different, the underlying memory is
> >>>>>>> +    /// still the same for these purposes (e.g., it's still a data race if they both write to the
> >>>>>>> +    /// same underlying byte at the same time).
> >>>>>>> +    fn with_pointer_into_page<T>(
> >>>>>>> +        &self,
> >>>>>>> +        off: usize,
> >>>>>>> +        len: usize,
> >>>>>>> +        f: impl FnOnce(*mut u8) -> Result<T>,
> >>>>>>
> >>>>>> I wonder whether the way to go here is making this function signature:
> >>>>>>
> >>>>>>        fn with_slice_in_page<T> (
> >>>>>>            &self,
> >>>>>> 	       off: usize,
> >>>>>> 	       len: usize,
> >>>>>> 	       f: iml FnOnce(&UnsafeCell<[u8]>) -> Result<T>
> >>>>>>        ) -> Result<T>
> >>>>>>
> >>>>>> , because in this way, it makes a bit more clear that what memory that
> >>>>>> `f` can access, in other words, the users are less likely to use the
> >>>>>> pointer in a wrong way.
> >>>>>>
> >>>>>> But that depends on whether `&UnsafeCell<[u8]>` is the correct
> >>>>>> abstraction and the ecosystem around it: for example, I feel like these
> >>>>>> two functions:
> >>>>>>
> >>>>>> 	    fn len(slice: &UnsafeCell<[u8]>) -> usize
> >>>>>> 	    fn as_ptr(slice: &UnsafeCell<[u8]>) -> *mut u8
> >>>>>>
> >>>>>> should be trivially safe, but I might be wrong. Again this is just for
> >>>>>> future discussion.
> >>>>>
> >>>>> I think the "better" type would be `&[UnsafeCell<u8>]`. Since there you
> >>>>> can always access the length.
> >>>>>
> >>>>
> >>>> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
> >>>> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,
> >>>> right? I haven't found any method doing that, but the length should be
> >>>> just a part of fat pointer, so I think getting that is a defined
> >>>> behavior. But maybe I'm missing something.
> >>
> >> There is `to_raw_parts` [1], but that is unstable. (Note that
> >> `<[T] as Pointee>::Metadata = usize`, see [2])
> >>
> > 
> > Oh, that's good to know, thank you! ;-)
> >
> >>> Hmm... but I guess one of the problems of this approach, is how to
> >>> construct a `&UnsafeCell<[u8]>` from a pointer and length...
> >>
> >> We could use `from_raw_parts` [3]. But when making the slice the outer
> >> type, we can use a stable function to convert a pointer and a length to
> >> a slice [4].
> >>
> > 
> > Yes, but there appears no way to get a pointer with larger provenance
> > from a `&[UnsafeCell<u8>]`, right?
> 
> What do you mean by "larger provenance"?
> 

Say you have a `&[UnsafeCell<u8>]` whose length is 64, what's the proper
way to get a `*mut u8` (or any other pointer) has the provenance for the
whole 64 bytes, so that you can pass it to a memcpy like function?
"larger" means the size of the provenance is larger than u8.

> >>>>> Another question would be if page allows for uninitialized bits, in that
> >>>>> case, we would need `&[Opaque<u8>]`.
> >>>>>
> >>>>
> >>>> Yes, or `&Opaque<[u8>]`.
> >>
> >> I don't think that putting the slice on the inside is what we want. Also
> > 
> > Hmm.. why? So in `&UnsafeCell<[u8]>` vs `&[UnsafeCell<u8>]` case, I
> > think the former represent "a slice of u8 that can be modified in the
> > same time" very well, and this is what a pointer-and-length pair usually
> > represents in kernel, I think. But yes, the latter is OK to me as well,
> > just hard to play the provenance game I guess?
> 
> Ultimately it again comes down to missing field projections :)
> 
> The type `&UnsafeCell<[u8]>` is less *useful*, since you cannot even get
> the length of the slice. Also indexing into this type is not easily
> possible. This is because the only way to get/change the inner value of
> an `UnsafeCell` is via `get`.
> 
> Compare this with the slice type. It allows getting the length, indexing
> into it (ie a form of field projections, if we consider slices as having
> a variable amount of fields).
> 
> All those issues would be solved by (good) field projections.
> 
> 
> Field projections also give a reason for why using `&[UnsafeCell<u8>]`
> is not really different from `&UnsafeCell<[u8]>`: At any point in time
> we ought to be able to project `&UnsafeCell<[u8]> -> &[UnsafeCell<u8>]`.
> 

Right, to me there is no significant difference between these two. Maybe
because I'm full field projected minded ;-)

> So it's fine to just use that from the get-go.
> 
> >> note that `Opaque<T>` requires that `T: Sized` and that is not the case
> >> for `[u8]`.
> > 
> > Oh, you're right. In case of MaybeUninit, it requires `T: Sized`, so
> > `Opaque<[u8]>` doesn't quite work.
> > 
> > Moving forward, maybe the first step is to see whether `&[Opaque<u8>]`
> > and `&[UnsafeCell<u8>]` can have a good way to generate a pointer with
> > proper provenance? Time to ping t-opsem maybe?
> 
> Good idea, do you want to do that, or should I do it?
> 

A way to get a larger provenance (explained above) is currently my only
question, so if you think that's something reasonable to ask, i.e.
nothing you know of can help. I will post a message there.

Regards,
Boqun

> -- 
> Cheers,
> Benno
>
Gary Guo April 25, 2024, 4:20 p.m. UTC | #9
On Thu, 18 Apr 2024 15:56:11 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:
 
> Hmm.. here is the thing, having `&UnsafeCell<[u8]>` means having a `*mut
> [u8]>`, and it should always be safe to get a "length" of `*mut [u8]`,  
> right? I haven't found any method doing that, but the length should be
> just a part of fat pointer, so I think getting that is a defined
> behavior. But maybe I'm missing something.

You can just use `unsafe_cell.get().len()`. `len` method exists on
raw slice pointers, gated by the `slice_ptr_len` feature, which will
be stable in 1.79.0.

Best,
Gary
diff mbox series

Patch

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ddb5644d4fd9..0862261cfbed 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -20,6 +20,7 @@ 
 
 /* `bindgen` gets confused at certain things. */
 const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
+const size_t RUST_CONST_HELPER_PAGE_SIZE = PAGE_SIZE;
 const gfp_t RUST_CONST_HELPER_GFP_ATOMIC = GFP_ATOMIC;
 const gfp_t RUST_CONST_HELPER_GFP_KERNEL = GFP_KERNEL;
 const gfp_t RUST_CONST_HELPER_GFP_KERNEL_ACCOUNT = GFP_KERNEL_ACCOUNT;
diff --git a/rust/helpers.c b/rust/helpers.c
index 312b6fcb49d5..72361003ba91 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -25,6 +25,8 @@ 
 #include <linux/build_bug.h>
 #include <linux/err.h>
 #include <linux/errname.h>
+#include <linux/gfp.h>
+#include <linux/highmem.h>
 #include <linux/mutex.h>
 #include <linux/refcount.h>
 #include <linux/sched/signal.h>
@@ -93,6 +95,24 @@  int rust_helper_signal_pending(struct task_struct *t)
 }
 EXPORT_SYMBOL_GPL(rust_helper_signal_pending);
 
+struct page *rust_helper_alloc_pages(gfp_t gfp_mask, unsigned int order)
+{
+	return alloc_pages(gfp_mask, order);
+}
+EXPORT_SYMBOL_GPL(rust_helper_alloc_pages);
+
+void *rust_helper_kmap_local_page(struct page *page)
+{
+	return kmap_local_page(page);
+}
+EXPORT_SYMBOL_GPL(rust_helper_kmap_local_page);
+
+void rust_helper_kunmap_local(const void *addr)
+{
+	kunmap_local(addr);
+}
+EXPORT_SYMBOL_GPL(rust_helper_kunmap_local);
+
 refcount_t rust_helper_REFCOUNT_INIT(int n)
 {
 	return (refcount_t)REFCOUNT_INIT(n);
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index f1c2c4aa22d2..7ab2b33f19d4 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -20,6 +20,13 @@ 
 #[derive(Clone, Copy)]
 pub struct Flags(u32);
 
+impl Flags {
+    /// Get the raw representation of this flag.
+    pub(crate) fn as_raw(self) -> u32 {
+        self.0
+    }
+}
+
 impl core::ops::BitOr for Flags {
     type Output = Self;
     fn bitor(self, rhs: Self) -> Self::Output {
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7ee807ae4680..048e1662829a 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -35,6 +35,7 @@ 
 pub mod kunit;
 #[cfg(CONFIG_NET)]
 pub mod net;
+pub mod page;
 pub mod prelude;
 pub mod print;
 mod static_assert;
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
new file mode 100644
index 000000000000..121d20066645
--- /dev/null
+++ b/rust/kernel/page.rs
@@ -0,0 +1,250 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! Kernel page allocation and management.
+
+use crate::{
+    alloc::{AllocError, Flags},
+    bindings,
+    error::code::*,
+    error::Result,
+    uaccess::UserSliceReader,
+};
+use core::ptr::{self, NonNull};
+
+/// A bitwise shift for the page size.
+pub const PAGE_SHIFT: usize = bindings::PAGE_SHIFT as usize;
+
+/// The number of bytes in a page.
+pub const PAGE_SIZE: usize = bindings::PAGE_SIZE;
+
+/// A bitmask that gives the page containing a given address.
+pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
+
+/// A pointer to a page that owns the page allocation.
+///
+/// # Invariants
+///
+/// The pointer is valid, and has ownership over the page.
+pub struct Page {
+    page: NonNull<bindings::page>,
+}
+
+// SAFETY: Pages have no logic that relies on them staying on a given thread, so moving them across
+// threads is safe.
+unsafe impl Send for Page {}
+
+// SAFETY: Pages have no logic that relies on them not being accessed concurrently, so accessing
+// them concurrently is safe.
+unsafe impl Sync for Page {}
+
+impl Page {
+    /// Allocates a new page.
+    ///
+    /// # Examples
+    ///
+    /// Allocate memory for a page.
+    ///
+    /// ```
+    /// use kernel::page::Page;
+    ///
+    /// # fn dox() -> Result<(), kernel::alloc::AllocError> {
+    /// let page = Page::alloc_page(GFP_KERNEL)?;
+    /// # Ok(()) }
+    /// ```
+    ///
+    /// Allocate memory for a page and zero its contents.
+    ///
+    /// ```
+    /// use kernel::page::Page;
+    ///
+    /// # fn dox() -> Result<(), kernel::alloc::AllocError> {
+    /// let page = Page::alloc_page(GFP_KERNEL | __GFP_ZERO)?;
+    /// # Ok(()) }
+    /// ```
+    pub fn alloc_page(flags: Flags) -> Result<Self, AllocError> {
+        // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
+        // is always safe to call this method.
+        let page = unsafe { bindings::alloc_pages(flags.as_raw(), 0) };
+        let page = NonNull::new(page).ok_or(AllocError)?;
+        // INVARIANT: We just successfully allocated a page, so we now have ownership of the newly
+        // allocated page. We transfer that ownership to the new `Page` object.
+        Ok(Self { page })
+    }
+
+    /// Returns a raw pointer to the page.
+    pub fn as_ptr(&self) -> *mut bindings::page {
+        self.page.as_ptr()
+    }
+
+    /// Runs a piece of code with this page mapped to an address.
+    ///
+    /// The page is unmapped when this call returns.
+    ///
+    /// # Using the raw pointer
+    ///
+    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
+    /// `PAGE_SIZE` bytes and for the duration in which the closure is called. The pointer might
+    /// only be mapped on the current thread, and when that is the case, dereferencing it on other
+    /// threads is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't
+    /// cause data races, the memory may be uninitialized, and so on.
+    ///
+    /// If multiple threads map the same page at the same time, then they may reference with
+    /// different addresses. However, even if the addresses are different, the underlying memory is
+    /// still the same for these purposes (e.g., it's still a data race if they both write to the
+    /// same underlying byte at the same time).
+    fn with_page_mapped<T>(&self, f: impl FnOnce(*mut u8) -> T) -> T {
+        // SAFETY: `page` is valid due to the type invariants on `Page`.
+        let mapped_addr = unsafe { bindings::kmap_local_page(self.as_ptr()) };
+
+        let res = f(mapped_addr.cast());
+
+        // This unmaps the page mapped above.
+        //
+        // SAFETY: Since this API takes the user code as a closure, it can only be used in a manner
+        // where the pages are unmapped in reverse order. This is as required by `kunmap_local`.
+        //
+        // In other words, if this call to `kunmap_local` happens when a different page should be
+        // unmapped first, then there must necessarily be a call to `kmap_local_page` other than the
+        // call just above in `with_page_mapped` that made that possible. In this case, it is the
+        // unsafe block that wraps that other call that is incorrect.
+        unsafe { bindings::kunmap_local(mapped_addr) };
+
+        res
+    }
+
+    /// Runs a piece of code with a raw pointer to a slice of this page, with bounds checking.
+    ///
+    /// If `f` is called, then it will be called with a pointer that points at `off` bytes into the
+    /// page, and the pointer will be valid for at least `len` bytes. The pointer is only valid on
+    /// this task, as this method uses a local mapping.
+    ///
+    /// If `off` and `len` refers to a region outside of this page, then this method returns
+    /// `EINVAL` and does not call `f`.
+    ///
+    /// # Using the raw pointer
+    ///
+    /// It is up to the caller to use the provided raw pointer correctly. The pointer is valid for
+    /// `len` bytes and for the duration in which the closure is called. The pointer might only be
+    /// mapped on the current thread, and when that is the case, dereferencing it on other threads
+    /// is UB. Other than that, the usual rules for dereferencing a raw pointer apply: don't cause
+    /// data races, the memory may be uninitialized, and so on.
+    ///
+    /// If multiple threads map the same page at the same time, then they may reference with
+    /// different addresses. However, even if the addresses are different, the underlying memory is
+    /// still the same for these purposes (e.g., it's still a data race if they both write to the
+    /// same underlying byte at the same time).
+    fn with_pointer_into_page<T>(
+        &self,
+        off: usize,
+        len: usize,
+        f: impl FnOnce(*mut u8) -> Result<T>,
+    ) -> Result<T> {
+        let bounds_ok = off <= PAGE_SIZE && len <= PAGE_SIZE && (off + len) <= PAGE_SIZE;
+
+        if bounds_ok {
+            self.with_page_mapped(move |page_addr| {
+                // SAFETY: The `off` integer is at most `PAGE_SIZE`, so this pointer offset will
+                // result in a pointer that is in bounds or one off the end of the page.
+                f(unsafe { page_addr.add(off) })
+            })
+        } else {
+            Err(EINVAL)
+        }
+    }
+
+    /// Maps the page and reads from it into the given buffer.
+    ///
+    /// This method will perform bounds checks on the page offset. If `offset .. offset+len` goes
+    /// outside ot the page, then this call returns `EINVAL`.
+    ///
+    /// # Safety
+    ///
+    /// * Callers must ensure that `dst` is valid for writing `len` bytes.
+    /// * Callers must ensure that this call does not race with a write to the same page that
+    ///   overlaps with this read.
+    pub unsafe fn read_raw(&self, dst: *mut u8, offset: usize, len: usize) -> Result {
+        self.with_pointer_into_page(offset, len, move |src| {
+            // SAFETY: If `with_pointer_into_page` calls into this closure, then
+            // it has performed a bounds check and guarantees that `src` is
+            // valid for `len` bytes.
+            //
+            // There caller guarantees that there is no data race.
+            unsafe { ptr::copy_nonoverlapping(src, dst, len) };
+            Ok(())
+        })
+    }
+
+    /// Maps the page and writes into it from the given buffer.
+    ///
+    /// This method will perform bounds checks on the page offset. If `offset .. offset+len` goes
+    /// outside ot the page, then this call returns `EINVAL`.
+    ///
+    /// # Safety
+    ///
+    /// * Callers must ensure that `src` is valid for reading `len` bytes.
+    /// * Callers must ensure that this call does not race with a read or write to the same page
+    ///   that overlaps with this write.
+    pub unsafe fn write_raw(&self, src: *const u8, offset: usize, len: usize) -> Result {
+        self.with_pointer_into_page(offset, len, move |dst| {
+            // SAFETY: If `with_pointer_into_page` calls into this closure, then it has performed a
+            // bounds check and guarantees that `dst` is valid for `len` bytes.
+            //
+            // There caller guarantees that there is no data race.
+            unsafe { ptr::copy_nonoverlapping(src, dst, len) };
+            Ok(())
+        })
+    }
+
+    /// Maps the page and zeroes the given slice.
+    ///
+    /// This method will perform bounds checks on the page offset. If `offset .. offset+len` goes
+    /// outside ot the page, then this call returns `EINVAL`.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that this call does not race with a read or write to the same page that
+    /// overlaps with this write.
+    pub unsafe fn fill_zero_raw(&self, offset: usize, len: usize) -> Result {
+        self.with_pointer_into_page(offset, len, move |dst| {
+            // SAFETY: If `with_pointer_into_page` calls into this closure, then it has performed a
+            // bounds check and guarantees that `dst` is valid for `len` bytes.
+            //
+            // There caller guarantees that there is no data race.
+            unsafe { ptr::write_bytes(dst, 0u8, len) };
+            Ok(())
+        })
+    }
+
+    /// Copies data from userspace into this page.
+    ///
+    /// This method will perform bounds checks on the page offset. If `offset .. offset+len` goes
+    /// outside ot the page, then this call returns `EINVAL`.
+    ///
+    /// Like the other `UserSliceReader` methods, data races are allowed on the userspace address.
+    /// However, they are not allowed on the page you are copying into.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that this call does not race with a read or write to the same page that
+    /// overlaps with this write.
+    pub unsafe fn copy_from_user_slice_raw(
+        &self,
+        reader: &mut UserSliceReader,
+        offset: usize,
+        len: usize,
+    ) -> Result {
+        self.with_pointer_into_page(offset, len, move |dst| {
+            // SAFETY: If `with_pointer_into_page` calls into this closure, then it has performed a
+            // bounds check and guarantees that `dst` is valid for `len` bytes. Furthermore, we have
+            // exclusive access to the slice since the caller guarantees that there are no races.
+            reader.read_raw(unsafe { core::slice::from_raw_parts_mut(dst.cast(), len) })
+        })
+    }
+}
+
+impl Drop for Page {
+    fn drop(&mut self) {
+        // SAFETY: By the type invariants, we have ownership of the page and can free it.
+        unsafe { bindings::__free_pages(self.page.as_ptr(), 0) };
+    }
+}