From: Andy Lutomirski <luto@kernel.org> Date: 2018-05-08 02:49:15
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref] wrote:
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which my
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even better,
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on power
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible, this
thing should work the same way on x86 and POWER.
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both. It should at least update the values loaded when a signal
is delivered and it should probably also update it for new threads.
For glibc, for example, I assume that you want signals to be delivered with
write access disabled to the GOT. Otherwise you would fail to protect
against exploits that occur in signal context. Glibc controls thread
creation, so the initial state on thread startup doesn't really matter, but
there will be more users than just glibc.
--Andy
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref] wrote:
quoted
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which my
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even better,
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on power
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible, this
thing should work the same way on x86 and POWER.
quoted
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both.
So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the
existing (different) behavior without the flag?
Ram, would you be okay with that? Could you give me a hand if
necessary? (I assume we have silicon in-house because it's a
long-standing feature of the POWER platform which was simply dormant on
Linux until now.)
It should at least update the values loaded when a signal
is delivered and it should probably also update it for new threads.
I think we should keep inheritance for new threads and fork. pkey_alloc
only has a single access rights argument, which makes it hard to reuse
this interface if there are two (three) separate sets of access rights.
Is there precedent for process state reverting on fork, besides
MADV_WIPEONFORK? My gut feeling is that we should avoid that.
For glibc, for example, I assume that you want signals to be delivered with
write access disabled to the GOT. Otherwise you would fail to protect
against exploits that occur in signal context. Glibc controls thread
creation, so the initial state on thread startup doesn't really matter, but
there will be more users than just glibc.
glibc does not control thread, or more precisely, subprocess creation.
Otherwise we wouldn't have face that many issues with our PID cache. 8-/
Thanks,
Florian
From: Andy Lutomirski <luto@kernel.org> Date: 2018-05-09 14:41:37
On Tue, May 8, 2018 at 5:40 AM Florian Weimer [off-list ref] wrote:
On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
quoted
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref]
wrote:
quoted
quoted
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which
my
quoted
quoted
quoted
quoted
quoted
quoted
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even better,
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on power
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible, this
thing should work the same way on x86 and POWER.
quoted
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both.
So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the
existing (different) behavior without the flag?
Ram, would you be okay with that? Could you give me a hand if
necessary? (I assume we have silicon in-house because it's a
long-standing feature of the POWER platform which was simply dormant on
Linux until now.)
quoted
It should at least update the values loaded when a signal
is delivered and it should probably also update it for new threads.
I think we should keep inheritance for new threads and fork. pkey_alloc
only has a single access rights argument, which makes it hard to reuse
this interface if there are two (three) separate sets of access rights.
Hmm. I can get on board with the idea that fork() / clone() /
pthread_create() are all just special cases of the idea that the thread
that *calls* them should have the right pkey values, and the latter is
already busted given our inability to asynchronously propagate the new mode
in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point.
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could add
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state *and* does the setsignal thing. If there are no keys left
with the correct initial state, then it fails.
Hmm. I can get on board with the idea that fork() / clone() /
pthread_create() are all just special cases of the idea that the thread
that*calls* them should have the right pkey values, and the latter is
already busted given our inability to asynchronously propagate the new mode
in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point.
Ram, any suggestions for implementing this on POWER?
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could add
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state*and* does the setsignal thing. If there are no keys left
with the correct initial state, then it fails.
The initial PKRU value can currently be configured by the system
administrator. I fear this approach has too many moving parts to be viable.
Thanks,
Florian
From: Andy Lutomirski <luto@amacapital.net> Date: 2018-05-14 15:33:00
On May 14, 2018, at 5:01 AM, Florian Weimer [off-list ref] wrote:
quoted
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could add
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state*and* does the setsignal thing. If there are no keys left
with the correct initial state, then it fails.
The initial PKRU value can currently be configured by the system administrator. I fear this approach has too many moving parts to be viable.
Honestly, I think we should drop that option. I don’t see how we can expect an administrator to do this usefully.
On May 14, 2018, at 5:01 AM, Florian Weimer [off-list ref] wrote:
quoted
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could add
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state*and* does the setsignal thing. If there are no keys left
with the correct initial state, then it fails.
The initial PKRU value can currently be configured by the system administrator. I fear this approach has too many moving parts to be viable.
Honestly, I think we should drop that option. I don’t see how we can expect an administrator to do this usefully.
I don't disagree—it makes things way less predictable in practice.
Thanks,
Florian
From: Dave Hansen <hidden> Date: 2018-05-16 17:02:02
On 05/14/2018 08:34 AM, Florian Weimer wrote:
quoted
quoted
The initial PKRU value can currently be configured by the system
administrator. I fear this approach has too many moving parts to be
viable.
Honestly, I think we should drop that option. I don’t see how we can
expect an administrator to do this usefully.
I don't disagree—it makes things way less predictable in practice.
I originally put that thing in there to make Andy happy with the initial
permissions, and give us a way to back it out if something went wrong.
I have no objections to removing it either.
On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
quoted
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref] wrote:
quoted
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which my
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even better,
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on power
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible, this
thing should work the same way on x86 and POWER.
quoted
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both.
So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the
existing (different) behavior without the flag?
Ram, would you be okay with that? Could you give me a hand if
necessary? (I assume we have silicon in-house because it's a
long-standing feature of the POWER platform which was simply dormant
on Linux until now.)
Yes. I can help you with that.
So let me see if I understand the overall idea.
Application can allocate new keys through a new syscall
sys_pkey_alloc_1(flags, init_val, sig_init_val)
'sig_init_val' is the permission-state of the key in signal context.
The kernel will set the permission of each keys to their
corresponding values when entering the signal handler and revert
on return from the signal handler.
just like init_val, sig_init_val also percolates to children threads.
Is this a correct understanding?
RP
From: Andy Lutomirski <luto@kernel.org> Date: 2018-05-16 20:38:00
On Wed, May 16, 2018 at 1:35 PM Ram Pai [off-list ref] wrote:
On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
quoted
On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
quoted
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref]
wrote:
quoted
quoted
quoted
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which
my
quoted
quoted
quoted
quoted
quoted
quoted
quoted
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even
better,
quoted
quoted
quoted
quoted
quoted
quoted
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on
power
quoted
quoted
quoted
quoted
quoted
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible,
this
quoted
quoted
quoted
quoted
thing should work the same way on x86 and POWER.
quoted
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both.
So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the
existing (different) behavior without the flag?
Ram, would you be okay with that? Could you give me a hand if
necessary? (I assume we have silicon in-house because it's a
long-standing feature of the POWER platform which was simply dormant
on Linux until now.)
Yes. I can help you with that.
So let me see if I understand the overall idea.
Application can allocate new keys through a new syscall
sys_pkey_alloc_1(flags, init_val, sig_init_val)
'sig_init_val' is the permission-state of the key in signal context.
The kernel will set the permission of each keys to their
corresponding values when entering the signal handler and revert
on return from the signal handler.
just like init_val, sig_init_val also percolates to children threads.
I was imagining it would be just pkey_alloc(SOME_NEW_FLAG, init_val); and
the init val would be used for the current thread and for signal handlers.
New threads would inherit their parents' values. The latter is certainly
up for negotiation, but it's the simplest behavior, and it's not obvious to
be that it's wrong.
--Andy
On Mon, May 14, 2018 at 02:01:23PM +0200, Florian Weimer wrote:
On 05/09/2018 04:41 PM, Andy Lutomirski wrote:
quoted
Hmm. I can get on board with the idea that fork() / clone() /
pthread_create() are all just special cases of the idea that the thread
that*calls* them should have the right pkey values, and the latter is
already busted given our inability to asynchronously propagate the new mode
in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point.
Ram, any suggestions for implementing this on POWER?
I suspect the changes will go in
restore_user_regs() and save_user_regs(). These are the functions
that save and restore register state before entry and exit into/from
a signal handler.
quoted
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could add
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state*and* does the setsignal thing. If there are no keys left
with the correct initial state, then it fails.
The initial PKRU value can currently be configured by the system
administrator. I fear this approach has too many moving parts to be
viable.
Sounds like on x86 keys can go active in signal-handler
without any explicit allocation request by the application. This is not
the case on power. Is that API requirement? Hope not.
RP
From: Andy Lutomirski <luto@kernel.org> Date: 2018-05-16 20:55:09
On Wed, May 16, 2018 at 1:52 PM Ram Pai [off-list ref] wrote:
On Mon, May 14, 2018 at 02:01:23PM +0200, Florian Weimer wrote:
quoted
On 05/09/2018 04:41 PM, Andy Lutomirski wrote:
quoted
Hmm. I can get on board with the idea that fork() / clone() /
pthread_create() are all just special cases of the idea that the thread
that*calls* them should have the right pkey values, and the latter is
already busted given our inability to asynchronously propagate the new
mode
quoted
quoted
in pkey_alloc(). So let's so PKEY_ALLOC_SETSIGNAL as a starting point.
Ram, any suggestions for implementing this on POWER?
I suspect the changes will go in
restore_user_regs() and save_user_regs(). These are the functions
that save and restore register state before entry and exit into/from
a signal handler.
quoted
quoted
One thing we could do, though: the current initual state on process
creation is all access blocked on all keys. We could change it so that
half the keys are fully blocked and half are read-only. Then we could
add
quoted
quoted
a PKEY_ALLOC_STRICT or similar that allocates a key with the correct
initial state*and* does the setsignal thing. If there are no keys
left
quoted
quoted
with the correct initial state, then it fails.
The initial PKRU value can currently be configured by the system
administrator. I fear this approach has too many moving parts to be
viable.
Sounds like on x86 keys can go active in signal-handler
without any explicit allocation request by the application. This is not
the case on power. Is that API requirement? Hope not.
On x86, signals are currently delivered with all keys locked all the way
down (except for the magic one we use to emulate no-read access). I would
hesitate to change this for existing applications.
On Wed, May 16, 2018 at 01:37:46PM -0700, Andy Lutomirski wrote:
On Wed, May 16, 2018 at 1:35 PM Ram Pai [off-list ref] wrote:
quoted
On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
quoted
On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
quoted
On Mon, May 7, 2018 at 2:48 AM Florian Weimer [off-list ref]
wrote:
quoted
quoted
quoted
quoted
On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
quoted
On Wed, May 2, 2018 at 7:11 PM Ram Pai [off-list ref] wrote:
quoted
On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
quoted
quoted
If I recall correctly, the POWER maintainer did express a strong
desire
quoted
quoted
quoted
back then for (what is, I believe) their current semantics, which
my
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
PKEY_ALLOC_SIGNALINHERIT patch implements for x86, too.
Ram, I really really don't like the POWER semantics. Can you give
some
quoted
quoted
quoted
quoted
justification for them? Does POWER at least have an atomic way for
userspace to modify just the key it wants to modify or, even
better,
quoted
quoted
quoted
quoted
quoted
quoted
quoted
special load and store instructions to use alternate keys?
quoted
I wouldn't call it POWER semantics. The way I implemented it on
power
quoted
quoted
quoted
quoted
quoted
quoted
lead to the semantics, given that nothing was explicitly stated
about how the semantics should work within a signal handler.
I think that this is further evidence that we should introduce a new
pkey_alloc() mode and deprecate the old. To the extent possible,
this
quoted
quoted
quoted
quoted
quoted
thing should work the same way on x86 and POWER.
quoted
Do you propose to change POWER or to change x86?
Sorry for being slow to reply. I propose to introduce a new
PKEY_ALLOC_something variant on x86 and POWER and to make the behavior
match on both.
So basically implement PKEY_ALLOC_SETSIGNAL for POWER, and keep the
existing (different) behavior without the flag?
Ram, would you be okay with that? Could you give me a hand if
necessary? (I assume we have silicon in-house because it's a
long-standing feature of the POWER platform which was simply dormant
on Linux until now.)
quoted
Yes. I can help you with that.
quoted
So let me see if I understand the overall idea.
quoted
Application can allocate new keys through a new syscall
sys_pkey_alloc_1(flags, init_val, sig_init_val)
quoted
'sig_init_val' is the permission-state of the key in signal context.
quoted
The kernel will set the permission of each keys to their
corresponding values when entering the signal handler and revert
on return from the signal handler.
quoted
just like init_val, sig_init_val also percolates to children threads.
I was imagining it would be just pkey_alloc(SOME_NEW_FLAG, init_val); and
the init val would be used for the current thread and for signal handlers.
what would change the key-permission-values enforced in signal-handler
context? Or can it never be changed, ones set through sys_pkey_alloc()?
I suppose key-permission-values change done in non-signal-handler context,
will not apply to those in signal-handler context.
Can the signal handler change the key-permission-values from the
signal-handler context?
RP
what would change the key-permission-values enforced in signal-handler
context? Or can it never be changed, ones set through sys_pkey_alloc()?
The access rights can only be set by pkey_alloc and are unchanged after
that (so we do not have to discuss whether the signal handler access
rights are per-thread or not).
I suppose key-permission-values change done in non-signal-handler context,
will not apply to those in signal-handler context.
Correct, that is the plan.
Can the signal handler change the key-permission-values from the
signal-handler context?
Yes, changes are possible. The access rights given to pkey_alloc only
specify the initial access rights when the signal handler is entered.
We need to decide if we should restore it on exit from the signal
handler. There is also the matter of siglongjmp, which currently does
not restore the current thread's access rights. In general, this might
be difficult to implement because of the limited space in jmp_buf.
Thanks,
Florian
So let me see if I understand the overall idea.
Application can allocate new keys through a new syscall
sys_pkey_alloc_1(flags, init_val, sig_init_val)
'sig_init_val' is the permission-state of the key in signal context.
I would keep the existing system call and just add a flag, say
PKEY_ALLOC_SETSIGNAL. If the current thread needs different access
rights, it can set those rights just after pkey_alloc returns. There is
no race that matters here, I think.
Thanks,
Florian