linux-kernel - Re: [PATCH 26/26] x86, pkeys: Documentation

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] [day] [month] [year] [list]

Message-ID: <CALCETrXpADZOyKDKGgkC8x-Q25mOpFeWw-CzUYc8MhU5900=tQ@mail.gmail.com>
Date:	Wed, 21 Oct 2015 16:22:46 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Dave Hansen <dave@...1.net>
Cc:	Ingo Molnar <mingo@...nel.org>, Kees Cook <keescook@...gle.com>,
	"x86@...nel.org" <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux-MM <linux-mm@...ck.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andy Lutomirski <luto@...nel.org>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH 26/26] x86, pkeys: Documentation

On Wed, Oct 21, 2015 at 12:11 PM, Dave Hansen <dave@...1.net> wrote:
> On 10/21/2015 11:55 AM, Andy Lutomirski wrote:
>> On Fri, Oct 16, 2015 at 8:12 AM, Dave Hansen <dave@...1.net> wrote:
>>> On 10/03/2015 12:27 AM, Ingo Molnar wrote:
>>>>  - Along similar considerations, also add a sys_pkey_query() system call to query
>>>>    the mapping of a specific pkey. (returns -EBADF or so if the key is not mapped
>>>>    at the moment.) This too could be vDSO accelerated in the future.
>>>>
>>>> I.e. something like:
>>>>
>>>>      unsigned long sys_pkey_alloc (unsigned long flags, unsigned long init_val)
>>>>      unsigned long sys_pkey_set   (int pkey, unsigned long new_val)
>>>>      unsigned long sys_pkey_get   (int pkey)
>>>>      unsigned long sys_pkey_free  (int pkey)
>>>
>>> The pkey_set() operation is going to get a wee bit interesting with signals.
>>>
>>> pkey_set() will modify the _current_ context's PKRU which includes the
>>> register itself and the kernel XSAVE buffer (if active).  But, since the
>>> PKRU state is saved/restored with the XSAVE state, we will blow away any
>>> state set during the signal.
>>>
>>> I _think_ the right move here is to either keep a 'shadow' version of
>>> PKRU inside the kernel (for each thread) and always update the task's
>>> XSAVE PKRU state when returning from a signal handler.  Or, _copy_ the
>>> signal's PKRU state in to the main process's PKRU state when returning
>>> from a signal.
>>
>> Ick.  Or we could just declare that signals don't affect the PKRU
>> state by default and mask it off in sigreturn.
>
> Yeah, I've been messing with it in a few forms and it's pretty ugly.
>
> I think it will be easier if we say the PKRU rights are not inherited by
> signals and changes during a signal are tossed out.  Signal handlers are
> special anyway and folks have to be careful writing them.

This is somewhat related to something I've been pondering in a
different context: fsbase and gsbase.

If a program changes fsbase using wrfsbase, should a signal handler
override it?  And should a change made in a signal handler carry over
after sigreturn?  Arguably, for fsbase and gsbase, the answer is no --
anyone use uses them for userspace threading (which is presumably why
they happened in the first place, even though userspace threading has
possibly dubious value) probably wants their context switches to stick
across signal invocations.

So I think that propagating PKRU into the signal handler and keeping
the in-register value on sigreturn by default is probably a reasonable
choice.

(OTOH, there's an argument for allowing programs to reset PKRU on
signal delivery: you could sort of arrange for signal handler to be
more privileged than the code that invokes them.  But that's doable
with some asm regardless.)

>
>> In fact, maybe we should add a general xfeature (or whatever it's
>> called these days) to the xstate in the signal context that controls
>> which pieces are restored.  Then user code can tweak it if needed in
>> signal handlers.
>
> Yeah, that's probably a good idea long-term.  We're only getting more
> and more things managed by XSAVE and it's going to be increasingly
> interesting to glue real semantics back on top.
>

Should we maybe extend copy_user_to_fpregs_zeroing to have a pair of
masks, where one mask indicates which features are copied and another
indicates which are preserved?  It looks like we already allow some
control over which bits are restored from sigcontext versus being
restored to their init state.

We might need to add some kind of extended ucontext area for this.  I
don't know if we're starting to run out of space.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/