Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 4/20/26 08:46, Kevin Brodsky wrote:
> On 17/04/2026 19:38, David Hildenbrand (Arm) wrote:
>> On 4/17/26 17:59, Kevin Brodsky wrote:
>>> The first two are not meant to be directly called, they're the
>>> arch-specific implementation of kpkeys_set_level() and
>>> kpkeys_restore_pkey_reg(), and those generic functions handle some
>>> generic logic.
>>>
>>> arch_kpkeys_enabled() is directly used in generic code, so I suppose it
>>> could be renamed to kpkeys_enabled()? It's actually implemented in an
>>> arch header so I wasn't too sure about it.
>> I was skimming over patch #13 and spotted:
>>
>> +void·__init·kpkeys_hardened_pgtables_init(void)
>> +{
>> +› if·(!arch_kpkeys_enabled())
>> +› › return;
>> +
>> +› static_branch_enable(&kpkeys_hardened_pgtables_key);
>> +}
>>
>> The arch_* there can just go IMHO.
>>
>> I'd also do it for the two ones used by the GUARD macros. If we don't
>> expect common code wrappers (arch_kpkeys_enabled() vs. kpkeys_enabled),
>> then the arch_ is unnecessary information -- IMHO
>
> Makes sense. I could just rename arch_kpkeys_enabled() to
> kpkeys_enabled(), but I'm thinking having an arch abstraction could be
> clearer, after looking into protecting sparse-vmemmap page tables. The
> new version would look like this:
>
> * :
> - arch_supports_kpkeys()
> - arch_supports_kpkeys_early() [can be called before features have
> been detected]
>
> * defines:
> - kpkeys_enabled() -> arch_supports_kpkeys()
> - kpkeys_hardened_pgtables_enabled() -> static key
> - kpkeys_hardened_pgtables_early_enabled() ->
> arch_supports_kpkeys_early() [called when setting up sparse-vmemmap,
> linear map, etc.]
>
> There is extra #ifdef'ing going on in , but
> doesn't need to worry about it. I think this might be
> easier to follow, I don't like too much having an interface function
> like kpkeys_enabled() defined in an arch header (not great for
> kernel-doc comments either). Any thoughts?
No strong opinion on the indirection as long as we don't call arch_
stuff from ordinary pkey user code :)
--
Cheers,
David
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 17/04/2026 19:38, David Hildenbrand (Arm) wrote:
> On 4/17/26 17:59, Kevin Brodsky wrote:
>> On 17/04/2026 16:37, David Hildenbrand (Arm) wrote:
>>> On 2/27/26 18:54, Kevin Brodsky wrote:
kpkeys is a simple framework to enable the use of protection keys
(pkeys) to harden the kernel itself. This patch introduces the basic
API in : a couple of functions to set and restore
the pkey register and macros to define guard objects.
kpkeys introduces a new concept on top of pkeys: the kpkeys level.
Each level is associated to a set of permissions for the pkeys
managed by the kpkeys framework. kpkeys_set_level(lvl) sets those
permissions according to lvl, and returns the original pkey
register, to be later restored by kpkeys_restore_pkey_reg(). To
start with, only KPKEYS_LVL_DEFAULT is available, which is meant
to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since
this is the only available pkey for now).
Because each architecture implementing pkeys uses a different
representation for the pkey register, and may reserve certain pkeys
for specific uses, support for kpkeys must be explicitly indicated
by selecting ARCH_HAS_KPKEYS and defining the following functions in
, in addition to the macros provided in
:
- arch_kpkeys_set_level()
- arch_kpkeys_restore_pkey_reg()
- arch_kpkeys_enabled()
>>> Another thing: why not simply drop the "arch_" stuff from these helpers?
>> The first two are not meant to be directly called, they're the
>> arch-specific implementation of kpkeys_set_level() and
>> kpkeys_restore_pkey_reg(), and those generic functions handle some
>> generic logic.
>>
>> arch_kpkeys_enabled() is directly used in generic code, so I suppose it
>> could be renamed to kpkeys_enabled()? It's actually implemented in an
>> arch header so I wasn't too sure about it.
> I was skimming over patch #13 and spotted:
>
> +void·__init·kpkeys_hardened_pgtables_init(void)
> +{
> +›if·(!arch_kpkeys_enabled())
> +›› return;
> +
> +›static_branch_enable(&kpkeys_hardened_pgtables_key);
> +}
>
> The arch_* there can just go IMHO.
>
> I'd also do it for the two ones used by the GUARD macros. If we don't
> expect common code wrappers (arch_kpkeys_enabled() vs. kpkeys_enabled),
> then the arch_ is unnecessary information -- IMHO
Makes sense. I could just rename arch_kpkeys_enabled() to
kpkeys_enabled(), but I'm thinking having an arch abstraction could be
clearer, after looking into protecting sparse-vmemmap page tables. The
new version would look like this:
* :
- arch_supports_kpkeys()
- arch_supports_kpkeys_early() [can be called before features have
been detected]
* defines:
- kpkeys_enabled() -> arch_supports_kpkeys()
- kpkeys_hardened_pgtables_enabled() -> static key
- kpkeys_hardened_pgtables_early_enabled() ->
arch_supports_kpkeys_early() [called when setting up sparse-vmemmap,
linear map, etc.]
There is extra #ifdef'ing going on in , but
doesn't need to worry about it. I think this might be
easier to follow, I don't like too much having an interface function
like kpkeys_enabled() defined in an arch header (not great for
kernel-doc comments either). Any thoughts?
- Kevin
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 4/17/26 17:59, Kevin Brodsky wrote:
> On 17/04/2026 16:37, David Hildenbrand (Arm) wrote:
>> On 2/27/26 18:54, Kevin Brodsky wrote:
>>> kpkeys is a simple framework to enable the use of protection keys
>>> (pkeys) to harden the kernel itself. This patch introduces the basic
>>> API in : a couple of functions to set and restore
>>> the pkey register and macros to define guard objects.
>>>
>>> kpkeys introduces a new concept on top of pkeys: the kpkeys level.
>>> Each level is associated to a set of permissions for the pkeys
>>> managed by the kpkeys framework. kpkeys_set_level(lvl) sets those
>>> permissions according to lvl, and returns the original pkey
>>> register, to be later restored by kpkeys_restore_pkey_reg(). To
>>> start with, only KPKEYS_LVL_DEFAULT is available, which is meant
>>> to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since
>>> this is the only available pkey for now).
>>>
>>> Because each architecture implementing pkeys uses a different
>>> representation for the pkey register, and may reserve certain pkeys
>>> for specific uses, support for kpkeys must be explicitly indicated
>>> by selecting ARCH_HAS_KPKEYS and defining the following functions in
>>> , in addition to the macros provided in
>>> :
>>>
>>> - arch_kpkeys_set_level()
>>> - arch_kpkeys_restore_pkey_reg()
>>> - arch_kpkeys_enabled()
>> Another thing: why not simply drop the "arch_" stuff from these helpers?
>
> The first two are not meant to be directly called, they're the
> arch-specific implementation of kpkeys_set_level() and
> kpkeys_restore_pkey_reg(), and those generic functions handle some
> generic logic.
>
> arch_kpkeys_enabled() is directly used in generic code, so I suppose it
> could be renamed to kpkeys_enabled()? It's actually implemented in an
> arch header so I wasn't too sure about it.
I was skimming over patch #13 and spotted:
+void·__init·kpkeys_hardened_pgtables_init(void)
+{
+› if·(!arch_kpkeys_enabled())
+› › return;
+
+› static_branch_enable(&kpkeys_hardened_pgtables_key);
+}
The arch_* there can just go IMHO.
I'd also do it for the two ones used by the GUARD macros. If we don't
expect common code wrappers (arch_kpkeys_enabled() vs. kpkeys_enabled),
then the arch_ is unnecessary information -- IMHO
--
Cheers,
David
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 17/04/2026 16:37, David Hildenbrand (Arm) wrote: > On 2/27/26 18:54, Kevin Brodsky wrote: >> kpkeys is a simple framework to enable the use of protection keys >> (pkeys) to harden the kernel itself. This patch introduces the basic >> API in : a couple of functions to set and restore >> the pkey register and macros to define guard objects. >> >> kpkeys introduces a new concept on top of pkeys: the kpkeys level. >> Each level is associated to a set of permissions for the pkeys >> managed by the kpkeys framework. kpkeys_set_level(lvl) sets those >> permissions according to lvl, and returns the original pkey >> register, to be later restored by kpkeys_restore_pkey_reg(). To >> start with, only KPKEYS_LVL_DEFAULT is available, which is meant >> to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since >> this is the only available pkey for now). >> >> Because each architecture implementing pkeys uses a different >> representation for the pkey register, and may reserve certain pkeys >> for specific uses, support for kpkeys must be explicitly indicated >> by selecting ARCH_HAS_KPKEYS and defining the following functions in >> , in addition to the macros provided in >> : >> >> - arch_kpkeys_set_level() >> - arch_kpkeys_restore_pkey_reg() >> - arch_kpkeys_enabled() > Another thing: why not simply drop the "arch_" stuff from these helpers? The first two are not meant to be directly called, they're the arch-specific implementation of kpkeys_set_level() and kpkeys_restore_pkey_reg(), and those generic functions handle some generic logic. arch_kpkeys_enabled() is directly used in generic code, so I suppose it could be renamed to kpkeys_enabled()? It's actually implemented in an arch header so I wasn't too sure about it. - Kevin
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 2/27/26 18:54, Kevin Brodsky wrote: > kpkeys is a simple framework to enable the use of protection keys > (pkeys) to harden the kernel itself. This patch introduces the basic > API in : a couple of functions to set and restore > the pkey register and macros to define guard objects. > > kpkeys introduces a new concept on top of pkeys: the kpkeys level. > Each level is associated to a set of permissions for the pkeys > managed by the kpkeys framework. kpkeys_set_level(lvl) sets those > permissions according to lvl, and returns the original pkey > register, to be later restored by kpkeys_restore_pkey_reg(). To > start with, only KPKEYS_LVL_DEFAULT is available, which is meant > to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since > this is the only available pkey for now). > > Because each architecture implementing pkeys uses a different > representation for the pkey register, and may reserve certain pkeys > for specific uses, support for kpkeys must be explicitly indicated > by selecting ARCH_HAS_KPKEYS and defining the following functions in > , in addition to the macros provided in > : > > - arch_kpkeys_set_level() > - arch_kpkeys_restore_pkey_reg() > - arch_kpkeys_enabled() Another thing: why not simply drop the "arch_" stuff from these helpers? -- Cheers, David
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 17/04/2026 14:00, David Hildenbrand (Arm) wrote: > On 4/15/26 17:50, Kevin Brodsky wrote: >> On 15/04/2026 15:00, David Hildenbrand (Arm) wrote: >>> On 2/27/26 18:54, Kevin Brodsky wrote: kpkeys is a simple framework to enable the use of protection keys (pkeys) to harden the kernel itself. This patch introduces the basic API in : a couple of functions to set and restore the pkey register and macros to define guard objects. kpkeys introduces a new concept on top of pkeys: the kpkeys level. Each level is associated to a set of permissions for the pkeys managed by the kpkeys framework. kpkeys_set_level(lvl) sets those permissions according to lvl, and returns the original pkey register, to be later restored by kpkeys_restore_pkey_reg(). To start with, only KPKEYS_LVL_DEFAULT is available, which is meant to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since this is the only available pkey for now). Because each architecture implementing pkeys uses a different representation for the pkey register, and may reserve certain pkeys for specific uses, support for kpkeys must be explicitly indicated by selecting ARCH_HAS_KPKEYS and defining the following functions in , in addition to the macros provided in : >>> I don't quite understand the reason for using levels. Levels sounds like >>> it would all be in some ordered fashion, where higher levels have access >>> to lower levels. >> That was originally the idea indeed, but in practice I don't expect >> levels to have a strict ordering, as it's not practical for composing >> features. >> >>> Think of that as a key that can unlock all "lower" locks, not just a >>> single lock. >>> >>> Then, the question is about the ordering once we introduce new >>> keys/locks. With two, it obviously doesn't matter :) >>> >>> So naturally I wonder whether levels is really the right abstraction >>> here, and why we are not simply using "distinct" keys, like >>> >>> KPKEY_DEFAULT >>> KPKEY_PGTABLE >>> KPKEY_SUPER_SECRET1 >>> KPKEY_SUPER_SECRET2 >>> >>> Is it because you want KPKEY_PGTABLE also be able to write to KPKEY_DEFAULT? >> Right, and in general a given level may be able to write to any number >> of pkeys. That's why I don't want to conflate pkeys and levels. Agreed >> that "level" might not be the clearest term though, since there's no >> strict ordering. > As discussed offline, maybe the right terminology to use here would be > something like a "context". > > You'd be activating/setting a context. > > KPKEY_CTX_DEFAULT > KPKEY_CTX_PGTABLE > KPKEY_CTX_SUPER_SECRET1 Sounds good to me, that's more accurate than "level" if it is possible to give access to arbitrary pkeys to each context, which is the current assumption. > What is accessible (and how) is defined for each context. For example, I > would assume that all context allow for read/write access to everything > that KPKEY_CTX_DEFAULT has access to. Most contexts would, although as I mentioned in the previous email, unprivileged contexts such as eBPF programs may be further restricted. - Kevin
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 4/15/26 17:50, Kevin Brodsky wrote: > On 15/04/2026 15:00, David Hildenbrand (Arm) wrote: >> On 2/27/26 18:54, Kevin Brodsky wrote: >>> kpkeys is a simple framework to enable the use of protection keys >>> (pkeys) to harden the kernel itself. This patch introduces the basic >>> API in : a couple of functions to set and restore >>> the pkey register and macros to define guard objects. >>> >>> kpkeys introduces a new concept on top of pkeys: the kpkeys level. >>> Each level is associated to a set of permissions for the pkeys >>> managed by the kpkeys framework. kpkeys_set_level(lvl) sets those >>> permissions according to lvl, and returns the original pkey >>> register, to be later restored by kpkeys_restore_pkey_reg(). To >>> start with, only KPKEYS_LVL_DEFAULT is available, which is meant >>> to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since >>> this is the only available pkey for now). >>> >>> Because each architecture implementing pkeys uses a different >>> representation for the pkey register, and may reserve certain pkeys >>> for specific uses, support for kpkeys must be explicitly indicated >>> by selecting ARCH_HAS_KPKEYS and defining the following functions in >>> , in addition to the macros provided in >>> : >> I don't quite understand the reason for using levels. Levels sounds like >> it would all be in some ordered fashion, where higher levels have access >> to lower levels. > > That was originally the idea indeed, but in practice I don't expect > levels to have a strict ordering, as it's not practical for composing > features. > >> Think of that as a key that can unlock all "lower" locks, not just a >> single lock. >> >> Then, the question is about the ordering once we introduce new >> keys/locks. With two, it obviously doesn't matter :) >> >> So naturally I wonder whether levels is really the right abstraction >> here, and why we are not simply using "distinct" keys, like >> >> KPKEY_DEFAULT >> KPKEY_PGTABLE >> KPKEY_SUPER_SECRET1 >> KPKEY_SUPER_SECRET2 >> >> Is it because you want KPKEY_PGTABLE also be able to write to KPKEY_DEFAULT? > > Right, and in general a given level may be able to write to any number > of pkeys. That's why I don't want to conflate pkeys and levels. Agreed > that "level" might not be the clearest term though, since there's no > strict ordering. As discussed offline, maybe the right terminology to use here would be something like a "context". You'd be activating/setting a context. KPKEY_CTX_DEFAULT KPKEY_CTX_PGTABLE KPKEY_CTX_SUPER_SECRET1 What is accessible (and how) is defined for each context. For example, I would assume that all context allow for read/write access to everything that KPKEY_CTX_DEFAULT has access to. -- Cheers, David
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 15/04/2026 15:00, David Hildenbrand (Arm) wrote: > On 2/27/26 18:54, Kevin Brodsky wrote: >> kpkeys is a simple framework to enable the use of protection keys >> (pkeys) to harden the kernel itself. This patch introduces the basic >> API in : a couple of functions to set and restore >> the pkey register and macros to define guard objects. >> >> kpkeys introduces a new concept on top of pkeys: the kpkeys level. >> Each level is associated to a set of permissions for the pkeys >> managed by the kpkeys framework. kpkeys_set_level(lvl) sets those >> permissions according to lvl, and returns the original pkey >> register, to be later restored by kpkeys_restore_pkey_reg(). To >> start with, only KPKEYS_LVL_DEFAULT is available, which is meant >> to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since >> this is the only available pkey for now). >> >> Because each architecture implementing pkeys uses a different >> representation for the pkey register, and may reserve certain pkeys >> for specific uses, support for kpkeys must be explicitly indicated >> by selecting ARCH_HAS_KPKEYS and defining the following functions in >> , in addition to the macros provided in >> : > I don't quite understand the reason for using levels. Levels sounds like > it would all be in some ordered fashion, where higher levels have access > to lower levels. That was originally the idea indeed, but in practice I don't expect levels to have a strict ordering, as it's not practical for composing features. > Think of that as a key that can unlock all "lower" locks, not just a > single lock. > > Then, the question is about the ordering once we introduce new > keys/locks. With two, it obviously doesn't matter :) > > So naturally I wonder whether levels is really the right abstraction > here, and why we are not simply using "distinct" keys, like > > KPKEY_DEFAULT > KPKEY_PGTABLE > KPKEY_SUPER_SECRET1 > KPKEY_SUPER_SECRET2 > > Is it because you want KPKEY_PGTABLE also be able to write to KPKEY_DEFAULT? Right, and in general a given level may be able to write to any number of pkeys. That's why I don't want to conflate pkeys and levels. Agreed that "level" might not be the clearest term though, since there's no strict ordering. > But how would you handle KPKEY_SUPER_SECRET1 and KPKEY_SUPER_SECRET2 then? Presumably those would also have access to KPKEY_DEFAULT. However, if you consider the reverse situation where the level is less privileged than the default (say an eBPF program), then write access to KPKEY_DEFAULT would not be granted. Also worth noting on the notion of level that POE2 will bring further per-level restrictions, besides which pkeys can be accessed. For instance, we could prevent an unprivileged level from executing certain instructions. This isn't in scope for this series, but this is a consideration in the design of the kpkeys abstractions. - Kevin
Re: [PATCH v6 01/30] mm: Introduce kpkeys
On 2/27/26 18:54, Kevin Brodsky wrote: > kpkeys is a simple framework to enable the use of protection keys > (pkeys) to harden the kernel itself. This patch introduces the basic > API in : a couple of functions to set and restore > the pkey register and macros to define guard objects. > > kpkeys introduces a new concept on top of pkeys: the kpkeys level. > Each level is associated to a set of permissions for the pkeys > managed by the kpkeys framework. kpkeys_set_level(lvl) sets those > permissions according to lvl, and returns the original pkey > register, to be later restored by kpkeys_restore_pkey_reg(). To > start with, only KPKEYS_LVL_DEFAULT is available, which is meant > to grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since > this is the only available pkey for now). > > Because each architecture implementing pkeys uses a different > representation for the pkey register, and may reserve certain pkeys > for specific uses, support for kpkeys must be explicitly indicated > by selecting ARCH_HAS_KPKEYS and defining the following functions in > , in addition to the macros provided in > : I don't quite understand the reason for using levels. Levels sounds like it would all be in some ordered fashion, where higher levels have access to lower levels. Think of that as a key that can unlock all "lower" locks, not just a single lock. Then, the question is about the ordering once we introduce new keys/locks. With two, it obviously doesn't matter :) So naturally I wonder whether levels is really the right abstraction here, and why we are not simply using "distinct" keys, like KPKEY_DEFAULT KPKEY_PGTABLE KPKEY_SUPER_SECRET1 KPKEY_SUPER_SECRET2 Is it because you want KPKEY_PGTABLE also be able to write to KPKEY_DEFAULT? But how would you handle KPKEY_SUPER_SECRET1 and KPKEY_SUPER_SECRET2 then? -- Cheers, David
