linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
       [not found]       ` <CALCETrUfO=vXg5rT-n=y8pLktcq5+ORvgpsOXCHG4GaugB3k2A@mail.gmail.com>
@ 2018-05-02 23:38         ` Ram Pai
  2018-05-07  9:47           ` Florian Weimer
  0 siblings, 1 reply; 16+ messages in thread
From: Ram Pai @ 2018-05-02 23:38 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Florian Weimer, linux-arch, Linux-MM, Linux API, X86 ML,
	Dave Hansen, linux-x86_64, linuxppc-dev

On Wed, May 02, 2018 at 09:18:11PM +0000, Andy Lutomirski wrote:
> On Wed, May 2, 2018 at 2:13 PM Ram Pai <linuxram@us.ibm.com> wrote:
> 
> 
> > > Ram, would you please comment?
> 
> > on POWER the pkey behavior will remain the same at entry or at exit from
> > the signal handler.  For eg:  if a key is read-disabled on entry into
> > the signal handler, and gets read-enabled in the signal handler, than it
> > will continue to be read-enabled on return from the signal handler.
> 
> > In other words, changes to key permissions persist across signal
> > boundaries.
> 
> I don't know about POWER's ISA, but this is crappy behavior.  If a thread
> temporarily grants itself access to a restrictive memory key and then gets
> a signal, the signal handler should *not* have access to that key.

This is a new requirement that I was not aware off. Its not documented
anywhere AFAICT.  Regardless of how the ISA behaves, its still a kernel
behavior that needs to be clearly defined.

-- 
Ram Pai

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-02 23:38         ` [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics Ram Pai
@ 2018-05-07  9:47           ` Florian Weimer
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-07  9:47 UTC (permalink / raw)
  To: Ram Pai, Andy Lutomirski
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, Dave Hansen,
	linux-x86_64, linuxppc-dev

On 05/03/2018 01:38 AM, Ram Pai wrote:
> This is a new requirement that I was not aware off. Its not documented
> anywhere AFAICT.

Correct.  All inheritance behavior was deliberately left unspecified.

I'm surprised about the reluctance to fix the x86 behavior.  Are there 
any applications at all for the current semantics?

I guess I can implement this particular glibc hardening on POWER only 
for now.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
       [not found]                   ` <927c8325-4c98-d7af-b921-6aafcf8fe992@redhat.com>
@ 2018-05-08  2:49                     ` Andy Lutomirski
  2018-05-08 12:40                       ` Florian Weimer
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2018-05-08  2:49 UTC (permalink / raw)
  To: Florian Weimer
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, Andrew Lutomirski, linuxppc-dev

On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com> wrote:

> On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
> > On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
> >
> >> On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
> >>>
> >>>> If I recall correctly, the POWER maintainer did express a strong
> > desire
> >>>> 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
> >>> 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?
> >
> >> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-08  2:49                     ` Andy Lutomirski
@ 2018-05-08 12:40                       ` Florian Weimer
  2018-05-09 14:41                         ` Andy Lutomirski
  2018-05-16 20:35                         ` Ram Pai
  0 siblings, 2 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-08 12:40 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, linuxppc-dev

On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
> On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> 
>> On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
>>> On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
>>>
>>>> On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
>>>>>
>>>>>> If I recall correctly, the POWER maintainer did express a strong
>>> desire
>>>>>> 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
>>>>> 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?
>>>
>>>> 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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-08 12:40                       ` Florian Weimer
@ 2018-05-09 14:41                         ` Andy Lutomirski
  2018-05-14 12:01                           ` Florian Weimer
  2018-05-16 20:35                         ` Ram Pai
  1 sibling, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2018-05-09 14:41 UTC (permalink / raw)
  To: Florian Weimer
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, Andrew Lutomirski, linuxppc-dev

On Tue, May 8, 2018 at 5:40 AM Florian Weimer <fweimer@redhat.com> wrote:

> On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
> > On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com>
wrote:
> >
> >> On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
> >>> On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
> >>>
> >>>> On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
> >>>>>
> >>>>>> If I recall correctly, the POWER maintainer did express a strong
> >>> desire
> >>>>>> 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
> >>>>> 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?
> >>>
> >>>> 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.

> 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.


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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-09 14:41                         ` Andy Lutomirski
@ 2018-05-14 12:01                           ` Florian Weimer
  2018-05-14 15:32                             ` Andy Lutomirski
  2018-05-16 20:52                             ` Ram Pai
  0 siblings, 2 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-14 12:01 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, linuxppc-dev

On 05/09/2018 04:41 PM, Andy Lutomirski wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-14 12:01                           ` Florian Weimer
@ 2018-05-14 15:32                             ` Andy Lutomirski
  2018-05-14 15:34                               ` Florian Weimer
  2018-05-16 20:52                             ` Ram Pai
  1 sibling, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2018-05-14 15:32 UTC (permalink / raw)
  To: Florian Weimer
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, Andy Lutomirski, linuxppc-dev




> On May 14, 2018, at 5:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-14 15:32                             ` Andy Lutomirski
@ 2018-05-14 15:34                               ` Florian Weimer
  2018-05-16 17:01                                 ` Dave Hansen
  0 siblings, 1 reply; 16+ messages in thread
From: Florian Weimer @ 2018-05-14 15:34 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, linuxram, Dave Hansen,
	linux-x86_64, Andy Lutomirski, linuxppc-dev

On 05/14/2018 05:32 PM, Andy Lutomirski wrote:
> 
> 
> 
>> On May 14, 2018, at 5:01 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-14 15:34                               ` Florian Weimer
@ 2018-05-16 17:01                                 ` Dave Hansen
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Hansen @ 2018-05-16 17:01 UTC (permalink / raw)
  To: Florian Weimer, Andy Lutomirski
  Cc: linux-arch, Linux API, X86 ML, linuxram, Linux-MM, linux-x86_64,
	Andy Lutomirski, linuxppc-dev

On 05/14/2018 08:34 AM, Florian Weimer wrote:
>>> 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-08 12:40                       ` Florian Weimer
  2018-05-09 14:41                         ` Andy Lutomirski
@ 2018-05-16 20:35                         ` Ram Pai
  2018-05-16 20:37                           ` Andy Lutomirski
  2018-05-17 10:11                           ` Florian Weimer
  1 sibling, 2 replies; 16+ messages in thread
From: Ram Pai @ 2018-05-16 20:35 UTC (permalink / raw)
  To: Florian Weimer
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, Dave Hansen,
	linux-x86_64, Andy Lutomirski, linuxppc-dev

On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
> On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
> >On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> >>On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
> >>>On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
> >>>
> >>>>On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
> >>>>>
> >>>>>>If I recall correctly, the POWER maintainer did express a strong
> >>>desire
> >>>>>>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
> >>>>>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?
> >>>
> >>>>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.
> 
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-16 20:35                         ` Ram Pai
@ 2018-05-16 20:37                           ` Andy Lutomirski
  2018-05-16 21:07                             ` Ram Pai
  2018-05-17 10:11                           ` Florian Weimer
  1 sibling, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2018-05-16 20:37 UTC (permalink / raw)
  To: linuxram
  Cc: Florian Weimer, linux-arch, Linux-MM, Linux API, X86 ML,
	Dave Hansen, linux-x86_64, Andrew Lutomirski, linuxppc-dev

On Wed, May 16, 2018 at 1:35 PM Ram Pai <linuxram@us.ibm.com> wrote:

> On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
> > On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
> > >On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com>
wrote:
> > >
> > >>On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
> > >>>On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
> > >>>
> > >>>>On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
> > >>>>>
> > >>>>>>If I recall correctly, the POWER maintainer did express a strong
> > >>>desire
> > >>>>>>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
> > >>>>>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?
> > >>>
> > >>>>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.
> >
> > 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-14 12:01                           ` Florian Weimer
  2018-05-14 15:32                             ` Andy Lutomirski
@ 2018-05-16 20:52                             ` Ram Pai
  2018-05-16 20:54                               ` Andy Lutomirski
  1 sibling, 1 reply; 16+ messages in thread
From: Ram Pai @ 2018-05-16 20:52 UTC (permalink / raw)
  To: Florian Weimer
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, Dave Hansen,
	linux-x86_64, Andy Lutomirski, linuxppc-dev

On Mon, May 14, 2018 at 02:01:23PM +0200, Florian Weimer wrote:
> On 05/09/2018 04:41 PM, Andy Lutomirski wrote:
> >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.

> 
> >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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-16 20:52                             ` Ram Pai
@ 2018-05-16 20:54                               ` Andy Lutomirski
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Lutomirski @ 2018-05-16 20:54 UTC (permalink / raw)
  To: linuxram
  Cc: Florian Weimer, linux-arch, Linux-MM, Linux API, X86 ML,
	Dave Hansen, linux-x86_64, Andrew Lutomirski, linuxppc-dev

On Wed, May 16, 2018 at 1:52 PM Ram Pai <linuxram@us.ibm.com> wrote:

> On Mon, May 14, 2018 at 02:01:23PM +0200, Florian Weimer wrote:
> > On 05/09/2018 04:41 PM, Andy Lutomirski wrote:
> > >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.

> >
> > >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.

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-16 20:37                           ` Andy Lutomirski
@ 2018-05-16 21:07                             ` Ram Pai
  2018-05-17 10:09                               ` Florian Weimer
  0 siblings, 1 reply; 16+ messages in thread
From: Ram Pai @ 2018-05-16 21:07 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Florian Weimer, linux-arch, Linux-MM, Linux API, X86 ML,
	Dave Hansen, linux-x86_64, linuxppc-dev

On Wed, May 16, 2018 at 01:37:46PM -0700, Andy Lutomirski wrote:
> On Wed, May 16, 2018 at 1:35 PM Ram Pai <linuxram@us.ibm.com> wrote:
> 
> > On Tue, May 08, 2018 at 02:40:46PM +0200, Florian Weimer wrote:
> > > On 05/08/2018 04:49 AM, Andy Lutomirski wrote:
> > > >On Mon, May 7, 2018 at 2:48 AM Florian Weimer <fweimer@redhat.com>
> wrote:
> > > >
> > > >>On 05/03/2018 06:05 AM, Andy Lutomirski wrote:
> > > >>>On Wed, May 2, 2018 at 7:11 PM Ram Pai <linuxram@us.ibm.com> wrote:
> > > >>>
> > > >>>>On Wed, May 02, 2018 at 09:23:49PM +0000, Andy Lutomirski wrote:
> > > >>>>>
> > > >>>>>>If I recall correctly, the POWER maintainer did express a strong
> > > >>>desire
> > > >>>>>>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
> > > >>>>>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?
> > > >>>
> > > >>>>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.
> > >
> > > 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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-16 21:07                             ` Ram Pai
@ 2018-05-17 10:09                               ` Florian Weimer
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-17 10:09 UTC (permalink / raw)
  To: Ram Pai, Andy Lutomirski
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, Dave Hansen,
	linux-x86_64, linuxppc-dev

On 05/16/2018 11:07 PM, Ram Pai wrote:

> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics
  2018-05-16 20:35                         ` Ram Pai
  2018-05-16 20:37                           ` Andy Lutomirski
@ 2018-05-17 10:11                           ` Florian Weimer
  1 sibling, 0 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-17 10:11 UTC (permalink / raw)
  To: Ram Pai
  Cc: linux-arch, Linux-MM, Linux API, X86 ML, Dave Hansen,
	linux-x86_64, Andy Lutomirski, linuxppc-dev

On 05/16/2018 10:35 PM, Ram Pai wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-05-17 10:11 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20180502132751.05B9F401F3041@oldenburg.str.redhat.com>
     [not found] ` <248faadb-e484-806f-1485-c34a72a9ca0b@intel.com>
     [not found]   ` <822a28c9-5405-68c2-11bf-0c282887466d@redhat.com>
     [not found]     ` <20180502211254.GA5863@ram.oc3035372033.ibm.com>
     [not found]       ` <CALCETrUfO=vXg5rT-n=y8pLktcq5+ORvgpsOXCHG4GaugB3k2A@mail.gmail.com>
2018-05-02 23:38         ` [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics Ram Pai
2018-05-07  9:47           ` Florian Weimer
     [not found]     ` <57459C6F-C8BA-4E2D-99BA-64F35C11FC05@amacapital.net>
     [not found]       ` <6286ba0a-7e09-b4ec-e31f-bd091f5940ff@redhat.com>
     [not found]         ` <CALCETrVrm6yGiv6_z7RqdeB-324RoeMmjpf1EHsrGOh+iKb7+A@mail.gmail.com>
     [not found]           ` <b2df1386-9df9-2db8-0a25-51bf5ff63592@redhat.com>
     [not found]             ` <CALCETrW_Dt-HoG4keFJd8DSD=tvyR+bBCFrBDYdym4GQbfng4A@mail.gmail.com>
     [not found]               ` <20180503021058.GA5670@ram.oc3035372033.ibm.com>
     [not found]                 ` <CALCETrXRQF08exQVZqtTLOKbC8Ywq5x4EYH_1D7r5v9bdOSwbg@mail.gmail.com>
     [not found]                   ` <927c8325-4c98-d7af-b921-6aafcf8fe992@redhat.com>
2018-05-08  2:49                     ` Andy Lutomirski
2018-05-08 12:40                       ` Florian Weimer
2018-05-09 14:41                         ` Andy Lutomirski
2018-05-14 12:01                           ` Florian Weimer
2018-05-14 15:32                             ` Andy Lutomirski
2018-05-14 15:34                               ` Florian Weimer
2018-05-16 17:01                                 ` Dave Hansen
2018-05-16 20:52                             ` Ram Pai
2018-05-16 20:54                               ` Andy Lutomirski
2018-05-16 20:35                         ` Ram Pai
2018-05-16 20:37                           ` Andy Lutomirski
2018-05-16 21:07                             ` Ram Pai
2018-05-17 10:09                               ` Florian Weimer
2018-05-17 10:11                           ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).