linux-kernel - Re: [RFC PATCH v2 1/1] x86/pkeys: update PKRU to enable pkey 0 before XSAVE

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: <34F412AB-F301-40A9-9252-BDB4BA7CC4DE@oracle.com>
Date: Fri, 22 Mar 2024 18:30:53 +0000
From: Aruna Ramakrishna <aruna.ramakrishna@...cle.com>
To: Ingo Molnar <mingo@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        Matthias Neugschwandtner <matthias.neugschwandtner@...cle.com>,
        Andrew
 Brownsword <andrew.brownsword@...cle.com>
Subject: Re: [RFC PATCH v2 1/1] x86/pkeys: update PKRU to enable pkey 0 before
 XSAVE



> On Mar 22, 2024, at 2:46 AM, Ingo Molnar <mingo@...nel.org> wrote:
> 
> Ok, this looks a lot saner than the first patch.
> 
> A couple of requests:
> 
> 1)
> 
> Please split out all the PKRU parameter passing interface changes into a 
> separate patch. Ie. split out patches that don't change any behavior, and 
> try to make the final feature-enabling (bug-fixing) patch as small and easy 
> to read as possible. Maybe even have 3 patches:
> 
>  - function interface changes
>  - helper function additions
>  - behavioral changes to signal handler pkru context
> 
> 2)
> 
> I do agree that isolation of sandboxed execution into a non-zero pkey might 
> make sense. But this really needs an actual testcase.
> 
> 3)
> 
> The semantics you've implemented for sigaltstacks are not the only possible 
> ones. In principle, a signal handler with its own stack might want to have 
> its own key(s) enabled. In a way a Linux signal handler is a mini-thread 
> created on the fly, with its own stack and its own attributes. Some thought 
> & analysis should go into which way to go here, and the best path should be 
> chosen. Fixing the SIGSEGV you observed should be a happy side effect of 
> other worthwile improvements.
> 
> Thanks,
> 
> Ingo

Thank you, Ingo!

I will split this patch into multiple patches when I send it out as a patch review
request next. And add a testcase.

I agree that this patch covers a very specific use case, and it probably raises
more questions than it answers. That’s why I sent it out as an RFC - because
I wasn’t sure if this was the best way to add this functionality, and I wanted the
experts to weigh in.

As Dave suggested, I can instead do wrpkru(0) to enable all pkeys before
XSAVE.

Thanks,
Aruna