On Sat, Dec 16, 2017 at 04:25:14PM +0100, Florian Weimer wrote:
> On 12/16/2017 04:09 PM, Ram Pai wrote:
>
> >>It still restores the PKRU register value upon
> >>regular exit from the signal handler, which I think is something we
> >>should keep.
> >
> >On x86, the pkru value is restored, on return from the signal handler,
> >to the value before the signal handler was called. right?
> >
> >In other words, if 'x' was the value when signal handler was called, it
> >will be 'x' when return from the signal handler.
> >
> >If correct, than it is consistent with the behavior on POWER.
>
> That's good to know. I tended to implement the same semantics on x86.
>
> >>I think we still should add a flag, so that applications can easily
> >>determine if a kernel has this patch. Setting up a signal handler,
> >>sending the signal, and thus checking for inheritance is a bit
> >>involved, and we'd have to do this in the dynamic linker before we
> >>can use pkeys to harden lazy binding. The flag could just be a
> >>no-op, apart from the lack of an EINVAL failure if it is specified.
> >
> >Sorry. I am little confused. What should I implement on POWER?
> >PKEY_ALLOC_SETSIGNAL semantics?
>
> No, we would add a flag, with a different name, and this patch only:
>
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index ec39f73..021f1d4 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -523,14 +523,17 @@ static int do_mprotect_pkey(unsigned long
> start, size_t l
> return do_mprotect_pkey(start, len, prot, pkey);
> }
>
> +#define PKEY_ALLOC_FLAGS ((unsigned long) (PKEY_ALLOC_SETSIGNAL))
> +
> SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags, unsigned long, init_val)
> {
> int pkey;
> int ret;
>
> - /* No flags supported yet. */
> - if (flags)
> + /* check for unsupported flags */
> + if (flags & ~PKEY_ALLOC_FLAGS)
> return -EINVAL;
> +
> /* check for unsupported init values */
> if (init_val & ~PKEY_ACCESS_MASK)
> return -EINVAL;
>
>
> This way, an application can specify the flag during key allocation,
> and knows that if the allocation succeeds, the kernel implements
> access rights inheritance in signal handlers. I think we need this
> so that applications which are incompatible with the earlier x86
> implementation of memory protection keys do not use them.
>
> With my second patch (not the first one implementing
> PKEY_ALLOC_SETSIGNAL), no further changes to architecture=specific
> code are needed, except for the definition of the flag in the header
> files.
Ok. Sounds like I do not have much to do. My patches in its current form
will continue to work and provide the semantics you envision.
>
> I'm open to a different way towards conveying this information to
> userspace. I don't want to probe for the behavior by sending a
> signal because that is quite involved and would also be visible in
> debuggers, confusing programmers.
I am fine with your proposal.
RP
