Re: [PATCH] pkeys: Introduce PKEY_ALLOC_SIGNALINHERIT and change signal semantics

[Andy Lutomirski <luto@kernel.org>]

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.