linux-kernel - Re: [PATCH v3 0/2] rust: page: Add support for 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: <Zz-ts0s3jHsNP73f@casper.infradead.org>
Date: Thu, 21 Nov 2024 22:01:23 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Boqun Feng <boqun.feng@...il.com>
Cc: Abdiel Janulgue <abdiel.janulgue@...il.com>,
	Alice Ryhl <aliceryhl@...gle.com>, rust-for-linux@...r.kernel.org,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...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, Kairui Song <ryncsn@...il.com>
Subject: Re: [PATCH v3 0/2] rust: page: Add support for existing struct page
 mappings

On Thu, Nov 21, 2024 at 11:12:30AM -0800, Boqun Feng wrote:
> On Thu, Nov 21, 2024 at 11:30:13AM +0200, Abdiel Janulgue wrote:
> > Hi Boqun, Matthew:
> > 
> > On 21/11/2024 02:24, Boqun Feng wrote:
> > > > > So if I understand correctly, what Abdiel needs here is a way to convert
> > > > > a virtual address to the corresponding page, would it make sense to just
> > > > > use folio in this case? Abdiel, what's the operation you are going to
> > > > > call on the page you get?
> > > > 
> > > > Yes that's basically it. The goal here is represent those existing struct
> > > > page within this rust Page abstraction but at the same time to avoid taking
> > > > over its ownership.
> > > > 
> > > > Boqun, Alice, should we reconsider Ownable and Owned trait again? :)
> > > > 
> > > 
> > > Could you use folio in your case? If so, we can provide a simple binding
> > > for folio which should be `AlwaysRefcounted`, and re-investigate how
> > > page should be wrapped.
> > > 
> > 
> > I'm not sure. Is there a way to get the struct folio from a vmalloc'd
> > address, e.g vmalloc_to_folio()?
> > 
> 
> I think you can use page_folio(vmalloc_to_page(..)) to get the folio,
> but one thing to notice is that folio is guaranteed to be a non-tail
> page, so if you want to do something later for the particular page (if
> it's a tail page), you will need to know the offset of the that page in
> folio. You can do something like below:

This is one of those things which will work today, but will stop
working in the future, and anyway will only appear to work for some
users.

For example, both vmalloc and slab allocations do not use the refcount
on the struct page for anything.  eg this will be a UAF (please excuse
me writing in C):

	char *a = kmalloc(256, GFP_KERNEL);
	struct page *page = get_page(virt_to_page(a));
	char *b = page_address(page) + offset_in_page(a);
	// a and b will now have the same bit pattern
	kfree(a);
	*b = 1;

Once you've called kfree(), slab is entitled to hand that memory out
to any other user of kmalloc().  This might actually work to protect
vmalloc() memory from going away under you, but I intend to change
vmalloc so that it won't work (nothing to do with this patch series,
rather an approach to make vmalloc more efficient).

One reason you're confused today is that we have a temporary ambiguity
around what "folio" actually means.  The original definition (ie mine) was
simply that it was a non-tail page.  We're moving towards the definition
Johannes wanted, which is that it's only the memdesc for anonymous &
file-backed memory [1].  So while vmalloc_to_folio() makes sense under
the original definition, it's an absurdity under the new definition.

So, Abdiel, why are you trying to add this?  What are you actually
trying to accomplish in terms of "I am writing a device driver for XXX
and I need to ..."?  You've been very evasive up to now.

[1] Actually Johannes wants to split them apart even further so that
anon & file memory have different types, and we may yet get there.
One step at a time.