Re: Re [PATCH v5 2/5] x86/pkeys: Add helper functions to update PKRU on sigframe

From: Aruna Ramakrishna
Date: Tue Jun 11 2024 - 10:06:16 EST




> On Jun 10, 2024, at 2:39 PM, jeffxu@xxxxxxxxxxxx wrote:
>
> The orig_pkru & init_pkru_value is quite difficult to understand.
>
> case 1> init_pkru: 00 (allow all)
> orig_pkru all cases => allow all
>
> case 2> init_pkru: 01 (disable all)
> Orig_pkru:
> allow all 00 => 00 allow all.
> disable all 01 => 01 disable all.
> disable write 10 => 00 allow all <--- *** odd ***
> disable all 11 => 01 disable all
>
> case 3> init pkru: 10 (disable write)
> allow all 00 => 00 allow all.
> disable all 01 => 00 (allow all) <----*** odd ***
> disable write 10 => 10 allow all
> disable all 11 => 10 disable write <--- *** odd ***
>
> case 4> init pkru: 11 (disable all)
> orig_pkru all cases => unchanged.
>
> set PKRU(0) seems to be better, easy to understand.
>

I’m not sure I follow.

The default init_pkru is 0x55555554 (enable only pkey 0). Let’s assume the application
sets up PKRU = 0x55555545 (i.e. enable only pkey 2). We want to set up the PKRU
to enable both pkey 0 and pkey 2, before the XSAVE, so that both the current stack as
well as the alternate signal stack are writable.

So with
write_pkru(orig_pkru & pkru_get_init_value());

It changes PKRU to 0x55555544 - enabling both pkey 0 and pkey 2.

After the XSAVE, it calls update_pkru_in_sigframe(), which overwrites this (new)
PKRU saved on the sigframe with orig_pkru, which is 0x55555545 in this example.

Setting PKRU to 0 would be simpler, it would enable all pkeys - 0 through 15 - which,
as Thomas pointed out, seems unnecessary. The application needs the pkey it
enabled for access to its own stack, and we need to enable pkey 0 under the hood
to enable access to the alternate signal stack.

Thanks,
Aruna