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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2DB720B0-0921-4912-8C5F-F0EDDF77845D@oracle.com>
Date: Tue, 11 Jun 2024 14:05:32 +0000
From: Aruna Ramakrishna <aruna.ramakrishna@...cle.com>
To: "jeffxu@...omium.org" <jeffxu@...omium.org>
CC: "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        Keith Lucas
	<keith.lucas@...cle.com>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>, Thomas
 Gleixner <tglx@...utronix.de>,
        "x86@...nel.org" <x86@...nel.org>,
        Andrew
 Brownsword <andrew.brownsword@...cle.com>,
        Matthias Neugschwandtner
	<matthias.neugschwandtner@...cle.com>,
        "jeffxu@...gle.com"
	<jeffxu@...gle.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "sroettger@...gle.com"
	<sroettger@...gle.com>,
        "jorgelo@...omium.org" <jorgelo@...omium.org>,
        "rick.p.edgecombe@...el.com" <rick.p.edgecombe@...el.com>
Subject: Re: Re [PATCH v5 2/5] x86/pkeys: Add helper functions to update PKRU
 on sigframe



> On Jun 10, 2024, at 2:39 PM, jeffxu@...omium.org 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