Re: [PATCH v3 0/2] rust: page: Add support for existing struct page mappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux