Thread (16 messages) flat view 16 messages, 3 authors, 2017-12-18

Re: pkeys: Support setting access rights for signal handlers

From: Ram Pai <hidden>
Date: 2017-12-16 17:20:36
Also in: linux-arch, linux-mm

On Sat, Dec 16, 2017 at 04:25:14PM +0100, Florian Weimer wrote:
quoted hunk ↗ jump to hunk
On 12/16/2017 04:09 PM, Ram Pai wrote:
quoted
quoted
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.
quoted
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help