linux-kernel - Re: [PATCH v3 2/2] rust: page: Extend support to existing struct page mappings

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: <CAH5fLghUb8dEV9GVtJj497cJ5sp4Gg7=qeijfnV_w4BNd70qxg@mail.gmail.com>
Date: Fri, 22 Nov 2024 08:55:42 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Jann Horn <jannh@...gle.com>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>, rust-for-linux@...r.kernel.org, 
	Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
	Wedson Almeida Filho <wedsonaf@...il.com>, Valentin Obst <kernel@...entinobst.de>, 
	open list <linux-kernel@...r.kernel.org>, Andrew Morton <akpm@...ux-foundation.org>, 
	"open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>, airlied@...hat.com
Subject: Re: [PATCH v3 2/2] rust: page: Extend support to existing struct page mappings

On Thu, Nov 21, 2024 at 9:18 PM Jann Horn <jannh@...gle.com> wrote:
>
> On Wed, Nov 20, 2024 at 11:56 PM Abdiel Janulgue
> <abdiel.janulgue@...il.com> wrote:
> > On 19/11/2024 19:07, Jann Horn wrote:
> > >> +    pub fn page_slice_to_page<'a>(page: &PageSlice) -> Result<&'a Self>
> > >
> > > Sorry, can you explain to me what the semantics of this are? Does this
> > > create a Page reference that is not lifetime-bound to the PageSlice?
> >
> > This creates a Page reference that is tied to the lifetime of the `C
> > struct page` behind the PageSlice buffer. Basically, it's just a cast
> > from the struct page pointer and does not own that resource.
>
> How is the Page reference tied to the lifetime of the C "struct page"?
>
> I asked some Rust experts to explain to me what this method signature
> expands to, and they added the following to the Rust docs:
>
> https://github.com/rust-lang/reference/blob/master/src/lifetime-elision.md
> ```
> fn other_args1<'a>(arg: &str) -> &'a str;             // elided
> fn other_args2<'a, 'b>(arg: &'b str) -> &'a str;      // expanded
> ```
>
> Basically, my understanding is that since you are explicitly
> specifying that the result should have lifetime 'a, but you are not
> specifying the lifetime of the parameter, the parameter is given a
> separate, unrelated lifetime by the compiler? Am I misunderstanding
> how this works, or is that a typo in the method signature?

No, you are correct. The signature is wrong and lets the caller pick
any lifetime they want, with no relation to the lifetime of the
underlying `struct page`.

>From a C perspective, what are the lifetime requirements of vmalloc_to_page?

> > >> +fn to_vec_with_allocator<A: Allocator>(val: &[u8]) -> Result<Vec<PageSlice, A>, AllocError> {
> > > Do I understand correctly that this can be used to create a kmalloc
> > > allocation whose pages can then basically be passed to
> > > page_slice_to_page()?
> > >
> > > FYI, the page refcount does not protect against UAF of slab
> > > allocations through new slab allocations of the same size. In other
> > > words: The slab allocator can internally recycle memory without going
> > > through the page allocator, and the slab allocator itself does not
> > > care about page refcounts.
> > >
> > > If the Page returned from calling page_slice_to_page() on the slab
> > > memory pages returned from to_vec_with_allocator() is purely usable as
> > > a borrow and there is no way to later grab a refcounted reference to
> > > it or pass it into a C function that assumes it can grab a reference
> > > to the page, I guess that works.
> >
> > Yes, I think that is the intent. I appreciate your help in pointing out
> > the issues with using refcounts in slab memory pages. As you can see,
> > page_slice_to_page() only returns a Page reference (not a refcounted
> > Page). Hopefully that addresses your concern?
>
> Does Rust also prevent safe code from invoking inc_ref() on the
> returned Page reference? Normally, the AlwaysRefCounted trait means
> that safe code can create an owned reference from a shared reference,
> right?

In principle, no, you could invoke inc_ref() directly. But the correct
way to do it would be to use ARef::from(my_ref) to obtain an actual
refcounted reference.

Alice