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: <0195fb77-c55a-40d5-8fe2-5844158f4f63@gmail.com>
Date: Fri, 22 Nov 2024 01:18:28 +0200
From: Abdiel Janulgue <abdiel.janulgue@...il.com>
To: Matthew Wilcox <willy@...radead.org>, Boqun Feng <boqun.feng@...il.com>
Cc: 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 22/11/2024 00:01, Matthew Wilcox wrote:
> 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.

Background behind this is that we need this for the nova rust driver [0].

We need an abstraction of struct page to construct a scatterlist which 
is needed for an internal firmware structure. Now most of pages needed 
there come from vmalloc_to_page() which, unlike the current rust Page 
abstraction, not allocated on demand but is an existing mapping.

Hope that clears things up!

Regards,
Abdiel

[0] https://rust-for-linux.com/nova-gpu-driver