Re: [PATCH RFC v9 12/25] mm: kpkeys: Protect regular page tables

On Tue, Sep 08, 2026 at 12:11:57PM +0200, Kevin Brodsky wrote:

On 08/09/2026 09:33, Mike Rapoport wrote:

quoted

On Mon, Sep 07, 2026 at 05:52:32PM +0200, Kevin Brodsky wrote:

quoted

On 07/09/2026 12:54, Mike Rapoport wrote:

quoted

quoted

[...]

 static inline struct ptdesc *pagetable_alloc_noprof(gfp_t gfp, unsigned int order)
 {
-	struct page *page = alloc_pages_noprof(gfp | __GFP_COMP, order);
+	struct page *page;
+
+	if (kpkeys_hardened_pgtables_enabled())
+		page = kpkeys_pgtable_alloc(gfp | __GFP_COMP, order);
+	else
+		page = alloc_pages_noprof(gfp | __GFP_COMP, order);

Can we make it a sequence rahter than a branch?

kpkeys_pgtable_alloc() does alloc_pages and then sets their pkeys, so I
think something like this should work:

	page = alloc_pages_noprof(gfp | __GFP_COMP, order);
	if (!page)
		return NULL;
	err = kpkeys_pgtable_alloc(page);
	if (err) {
		__free_pages(page, order);
		return NULL;

with if (kpkeys_hardened_pgtables_enabled()) folded into
kpkeys_pgtable_alloc().

I agree this would be less ugly. In fact this is pretty much what this
series did up to RFC v5 (albeit in the ctor/dtor instead of alloc/free).
We could go back to this API, *but* the big issue is that it makes it
impossible to use a smarter allocation strategy for protected pages.
Patch 14 in RFC v6 [1] will give you an idea of what such an allocator
(with support for large blocks and splitting) would look like.

As discussed with David H we're first trying to land this feature
without large block support (fully PTE-mapped direct map), but we do
want to support large blocks eventually [2] and I would prefer the core
API to be already compatible with that objective. Very happy to hear
about suggestions as to how to avoid the explicit condition in
pagetable_alloc() though!

We could unconditionally replace alloc_pages_noprof() with
kpkeys_pgtable_alloc() and make the latter choose the right allocator, but
that's not very nice either :/

Yep I also thought of that but the naming would be really misleading...

Maybe, maybe, introduce __pagetable_alloc() that is a simple static
inline calling alloc_pages_noprof() if !CONFIG_KPKEYS_HARDENED_PGTABLES,
and otherwise checks kpkeys_hardened_pgtables_enabled()? Less ugliness
in pagetable_alloc() but even more further down, not sure this helps.

You are right and I can't say I have any bright ideas.
 

- Kevin

-- 
Sincerely yours,
Mike.