Re: [RFC PATCH v5 13/18] mm: Map page tables with privileged pkey

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 01/10/2025 17:28, David Hildenbrand wrote:
> On 15.08.25 10:55, Kevin Brodsky wrote:
>
> Just wondering, should the patch subject be:
>
> "mm: protect page tables with privileged pkey" ?
>
> At least patch #2 tells me that set_memory_pkey() will set the
> protection key, and the function is called
> "kpkeys_protect_pgtable_memory"?
>
> Just trying to connect the dots here :)

That's fair! I suppose I meant "map with privileged pkey" in the sense
of "modify the mapping/PTE to set the pkey". But I see how that can be
confusing, as we're not creating any mapping. I'll reword that in v6.

>
>> If CONFIG_KPKEYS_HARDENED_PGTABLES is enabled, map allocated page
>> table pages using a privileged pkey (KPKEYS_PKEY_PGTABLES), so that
>> page tables can only be written under guard(kpkeys_hardened_pgtables).
>>
>> This patch is a no-op if CONFIG_KPKEYS_HARDENED_PGTABLES is disabled
>> (default).
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>>   include/linux/mm.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index d9371d992033..4880cb7a4cb9 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -34,6 +34,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/cacheinfo.h>
>>   #include <linux/rcuwait.h>
>> +#include <linux/kpkeys.h>
>>     struct mempolicy;
>>   struct anon_vma;
>> @@ -2979,6 +2980,8 @@ static inline bool __pagetable_ctor(struct
>> ptdesc *ptdesc)
>>         __folio_set_pgtable(folio);
>>       lruvec_stat_add_folio(folio, NR_PAGETABLE);
>> +    if (kpkeys_protect_pgtable_memory(folio))
>> +        return false;
>>       return true;
>>   }
>>   @@ -2989,6 +2992,7 @@ static inline void pagetable_dtor(struct
>> ptdesc *ptdesc)
>>       ptlock_free(ptdesc);
>>       __folio_clear_pgtable(folio);
>>       lruvec_stat_sub_folio(folio, NR_PAGETABLE);
>> +    kpkeys_unprotect_pgtable_memory(folio);
>
> This is all rather nasty. Not your fault.
>
> In the near future page tables will not be folios, and the whole
> ptdesc_folio() conversion will not make any sense.

Ah, interesting. Any patches/series I should be aware of?

>
> Likely you should make kpkeys_protect_pgtable_memory() etc. consume an
> address range, or a page range right from the start.

Got it. That said, as per the discussion on the cover letter [1], it's
quite likely that we'll have to change the approach completely anyway
(i.e. allocate PTPs from a dedicated pool where the pkey is already set).

- Kevin

[1] lore.kernel.org/r/aMwd7IJVECEy8mzf@willie-the-truck