lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALNs47uy5rQ15wByzQA0_YzORM0nTFdi9-TvwyC4+ZTXVQBj4g@mail.gmail.com>
Date: Thu, 1 Feb 2024 01:50:53 -0500
From: Trevor Gross <tmgross@...ch.edu>
To: Boqun Feng <boqun.feng@...il.com>, Alice Ryhl <aliceryhl@...gle.com>, 
	Andreas Hindborg <a.hindborg@...sung.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Wedson Almeida Filho <wedsonaf@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Kees Cook <keescook@...omium.org>, 
	Al Viro <viro@...iv.linux.org.uk>, Andrew Morton <akpm@...ux-foundation.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Arve Hjønnevåg <arve@...roid.com>, 
	Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>, 
	Joel Fernandes <joel@...lfernandes.org>, Carlos Llamas <cmllamas@...gle.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Arnd Bergmann <arnd@...db.de>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH 3/3] rust: add abstraction for `struct page`

On Fri, Jan 26, 2024 at 1:28 PM Boqun Feng <boqun.feng@...il.com> wrote:
>
> On Fri, Jan 26, 2024 at 01:33:46PM +0100, Alice Ryhl wrote:
> > On Fri, Jan 26, 2024 at 1:47 AM Boqun Feng <boqun.feng@...il.com> wrote:
> > >
> > > On Wed, Jan 24, 2024 at 11:20:23AM +0000, Alice Ryhl wrote:
> > > > [...]
> > > > +    /// Maps the page and writes into it from the given buffer.
> > > > +    ///
> > > > +    /// # Safety
> > > > +    ///
> > > > +    /// Callers must ensure that `src` is valid for reading `len` bytes.
> > > > +    pub unsafe fn write(&self, src: *const u8, offset: usize, len: usize) -> Result {
> > >
> > > Use a slice like type as `src` maybe? Then the function can be safe:
> > >
> > >         pub fn write<S: AsRef<[u8]>>(&self, src: S, offset: usize) -> Result
> > >
> > > Besides, since `Page` impl `Sync`, shouldn't this `write` and the
> > > `fill_zero` be a `&mut self` function? Or make them both `unsafe`
> > > because of potential race and add some safety requirement?
> >
> > Ideally, we don't want data races with these methods to be UB. They
>
> I understand that, but in the current code, you can write:
>
>         CPU 0                   CPU 1
>         =====                   =====
>
>         page.write(src1, 0, 8);
>                                 page.write(src2, 0, 8);
>
> and it's a data race at kernel end. So my question is more how we can
> prevent the UB ;-)

Hm. Would the following work?

    // Change existing functions to work with references, meaning they need an
    // exclusive &mut self
    pub fn with_page_mapped<T>(
        &mut self,
        f: impl FnOnce(&mut [u8; PAGE_SIZE]) -> T
    ) -> T

    pub fn with_pointer_into_page<T>(
        &mut self,
        off: usize,
        len: usize,
        f: impl FnOnce(&mut [u8]) -> Result<T>,
    ) -> Result<T>

    // writing methods now take &mut self
    pub fn write(&mut self ...)
    pub fn fill_zero(&mut self ...)
    pub fn copy_into_page(&mut self ...)

    // Add two new functions that take &self, but return shared access
    pub fn with_page_mapped_raw<T>(
        &self,
        f: impl FnOnce(&UnsafeCell<[u8; PAGE_SIZE]>) -> T
    ) -> T

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

This would mean that anyone who can obey rust's mutability rules can
use a page without any safety or race conditions to worry about, much
better for usability.

But if you do need to allow the data to be shared and racy, such as
the userspace example, the `_raw` methods allow for that and you can
`.get()` a `*mut u8` from the UnsafeCell. This moves the interior
mutability only to the mapped data rather than the Page itself, which
I think is more accurate anyway.

Leveraging UnsafeCell would also make some things with UserSlicePtr
more clear too.

- Trevor

> Regards,
> Boqun
>
> > could be mapped into the address space of a userspace process.
> >
> > Alice
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ