linux-kernel - Re: [PATCH v3 4/4] rust: add abstraction for `struct page`

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: <ZfW7J2uS28hlFWcd@casper.infradead.org>
Date: Sat, 16 Mar 2024 15:30:47 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Andreas Hindborg <a.hindborg@...sung.com>, akpm@...ux-foundation.org,
	alex.gaynor@...il.com, arnd@...db.de, arve@...roid.com,
	benno.lossin@...ton.me, bjorn3_gh@...tonmail.com,
	boqun.feng@...il.com, brauner@...nel.org, cmllamas@...gle.com,
	gary@...yguo.net, gregkh@...uxfoundation.org,
	joel@...lfernandes.org, keescook@...omium.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, maco@...roid.com,
	ojeda@...nel.org, rust-for-linux@...r.kernel.org, surenb@...gle.com,
	tkjos@...roid.com, viro@...iv.linux.org.uk, wedsonaf@...il.com
Subject: Re: [PATCH v3 4/4] rust: add abstraction for `struct page`

On Mon, Mar 11, 2024 at 10:50:56AM +0000, Alice Ryhl wrote:
> Alice Ryhl <aliceryhl@...gle.com> writes:
> > +/// Flags for the "get free page" function that underlies all memory allocations.
> > +pub mod flags {
> > +    pub type gfp_t = bindings::gfp_t;
> > +
> > +    /// `GFP_KERNEL` is typical for kernel-internal allocations. The caller requires `ZONE_NORMAL`
> > +    /// or a lower zone for direct access but can direct reclaim.
> > +    pub const GFP_KERNEL: gfp_t = bindings::GFP_KERNEL;
> > +    /// `GFP_ZERO` returns a zeroed page on success.
> > +    pub const __GFP_ZERO: gfp_t = bindings::__GFP_ZERO;
> > +    /// `GFP_HIGHMEM` indicates that the allocated memory may be located in high memory.
> > +    pub const __GFP_HIGHMEM: gfp_t = bindings::__GFP_HIGHMEM;
> > +}
> >
> > [...]
> >
> > +impl Page {
> > +    /// Allocates a new page.
> > +    pub fn alloc_page(gfp_flags: flags::gfp_t) -> Result<Self, AllocError> {
> > +        // SAFETY: The specified order is zero and we want one page.
> > +        let page = unsafe { bindings::alloc_pages(gfp_flags, 0) };
> > +        let page = NonNull::new(page).ok_or(AllocError)?;
> > +        // INVARIANT: We checked that the allocation succeeded.
> > +        Ok(Self { page })
> > +    }
> 
> Matthew Wilcox: You suggested on a previous version that I use gfp flags
> here, or that I rename it to e.g. BinderPage to make it clear that this
> is specific to the kind of pages that Binder needs.

I think what you have here is good.

> In this version I added some gfp flags, but I'm not actually sure that
> the Page abstraction works for all combinations of gfp flags. For
> example, I use kmap_local_page when accessing the page, but is that
> correct if there's a user that doesn't pass GFP_HIGHMEM?

Yes, kmap_local_page() works for non-highmem pages (it's essentially a
no-op)