Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

4 messages, 3 authors, 2017-11-07 · open the first message on its own page

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

From: Florian Weimer <hidden>
Date: 2017-11-07 07:32:37

* Ram Pai:
On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
quoted
* Ram Pai:
=20
quoted
Testing:
-------
This patch series has passed all the protection key
tests available in the selftest directory.The
tests are updated to work on both x86 and powerpc.
The selftests have passed on x86 and powerpc hardware.
=20
How do you deal with the key reuse problem?  Is it the same as x86-64,
where it's quite easy to accidentally grant existing threads access to
a just-allocated key, either due to key reuse or a changed init_pkru
parameter?
I am not sure how on x86-64, two threads get allocated the same key
at the same time? the key allocation is guarded under the mmap_sem
semaphore. So there cannot be a race where two threads get allocated
the same key.
The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
sequence.  The pthread_create call makes the new thread inherit the
access rights of the current thread, but then the key is deallocated.
Reallocation of the same key will have that thread retain its access
rights, which is IMHO not correct.
Can you point me to the issue, if it is already discussed somewhere?
See =E2=80=98MPK: pkey_free and key reuse=E2=80=99 on various lists (includ=
ing
linux-mm and linux-arch).

It has a test case attached which demonstrates the behavior.
As far as the semantics is concerned, a key allocated in one thread's
context has no meaning if used in some other threads context within the
same process.  The app should not try to re-use a key allocated in a
thread's context in some other threads's context.
Uh-oh, that's not how this feature works on x86-64 at all.  There, the
keys are a process-global resource.  Treating them per-thread
seriously reduces their usefulness.
quoted
What about siglongjmp from a signal handler?
On powerpc there is some relief.  the permissions on a key can be
modified from anywhere, including from the signal handler, and the
effect will be immediate.  You dont have to wait till the
signal handler returns for the key permissions to be restore.
My concern is that the signal handler knows nothing about protection
keys, but the current x86-64 semantics will cause it to clobber the
access rights of the current thread.
also after return from the sigsetjmp();
possibly caused by siglongjmp(), the program can restore the permission
on any key.
So that's not really an option.
Atleast that is my theory. Can you give me a testcase; if you have one
handy.
The glibc patch I posted under the =E2=80=98MPK: pkey_free and key reuse=E2=
=80=99
thread covers this, too.

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

From: Ram Pai <hidden>
Date: 2017-11-07 22:40:09

On Tue, Nov 07, 2017 at 08:32:16AM +0100, Florian Weimer wrote:
* Ram Pai:
quoted
On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
quoted
* Ram Pai:
quoted
Testing:
-------
This patch series has passed all the protection key
tests available in the selftest directory.The
tests are updated to work on both x86 and powerpc.
The selftests have passed on x86 and powerpc hardware.
How do you deal with the key reuse problem?  Is it the same as x86-64,
where it's quite easy to accidentally grant existing threads access to
a just-allocated key, either due to key reuse or a changed init_pkru
parameter?
I am not sure how on x86-64, two threads get allocated the same key
at the same time? the key allocation is guarded under the mmap_sem
semaphore. So there cannot be a race where two threads get allocated
the same key.
The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
sequence.  The pthread_create call makes the new thread inherit the
access rights of the current thread, but then the key is deallocated.
Reallocation of the same key will have that thread retain its access
rights, which is IMHO not correct.
(Dave Hansen: please correct me if I miss-speak below)

As per the current semantics of sys_pkey_free(); the way I understand it,
the calling thread is saying disassociate me from this key. Other
threads continue to be associated with the key and could continue to
get key-faults, but this calling thread will not get key-faults on that
key any more.

Also the key should not get reallocated till all the threads in the process
have disassocated from the key; by calling sys_pkey_free().
From that point of view, I think there is a bug in the implementation of
pkey on x86 and now on powerpc aswell.
quoted
Can you point me to the issue, if it is already discussed somewhere?
See ‘MPK: pkey_free and key reuse’ on various lists (including
linux-mm and linux-arch).

It has a test case attached which demonstrates the behavior.
quoted
As far as the semantics is concerned, a key allocated in one thread's
context has no meaning if used in some other threads context within the
same process.  The app should not try to re-use a key allocated in a
thread's context in some other threads's context.
Uh-oh, that's not how this feature works on x86-64 at all.  There, the
keys are a process-global resource.  Treating them per-thread
seriously reduces their usefulness.
Sorry. I was not thinking right. Let me restate.

A key is a global resource, but the permissions on a key is
local to a thread. For eg: the same key could disable
access on a page for one thread, while it could disable write
on the same page on another thread.
quoted
quoted
What about siglongjmp from a signal handler?
On powerpc there is some relief.  the permissions on a key can be
modified from anywhere, including from the signal handler, and the
effect will be immediate.  You dont have to wait till the
signal handler returns for the key permissions to be restore.
My concern is that the signal handler knows nothing about protection
keys, but the current x86-64 semantics will cause it to clobber the
access rights of the current thread.
quoted
also after return from the sigsetjmp();
possibly caused by siglongjmp(), the program can restore the permission
on any key.
So that's not really an option.
quoted
Atleast that is my theory. Can you give me a testcase; if you have one
handy.
The glibc patch I posted under the ‘MPK: pkey_free and key reuse’
thread covers this, too.
thanks. will try the test case with my kernel patches. But, on
powerpc one can change the permissions on the key in the signal handler
which takes into effect immediately, there should not be a bug
in powerpc.

x86 has this requirement where it has to return from the signal handler
back to the kernel in order to change the permission on a key,
it can cause issues with longjump.

RP

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

From: Dave Hansen <hidden>
Date: 2017-11-07 22:47:16

On 11/07/2017 02:39 PM, Ram Pai wrote:
As per the current semantics of sys_pkey_free(); the way I understand it,
the calling thread is saying disassociate me from this key.
No.  It is saying: "this *process* no longer has any uses of this key,
it can be reused".

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

From: Ram Pai <hidden>
Date: 2017-11-07 23:44:43

On Tue, Nov 07, 2017 at 02:47:10PM -0800, Dave Hansen wrote:
On 11/07/2017 02:39 PM, Ram Pai wrote:
quoted
As per the current semantics of sys_pkey_free(); the way I understand it,
the calling thread is saying disassociate me from this key.
No.  It is saying: "this *process* no longer has any uses of this key,
it can be reused".
ok, in light of the corrected semantics, I see no bug in the implimentation.
On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
...
The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
sequence.  The pthread_create call makes the new thread inherit the
access rights of the current thread, but then the key is deallocated.
Reallocation of the same key will have that thread retain its access
rights, which is IMHO not correct.
Again.. in light of the corrected semantics --
 the child thread or any thread should not free
a key without cleaning up. 
(a) disassociate the key from its address space
(b) reset the permission on the key across all the threads of the
process.

Because any such uncleaned bits can cause unexpected behavior if the 
same key gets reallocated on sys_pkey_alloc().


-- 
Ram Pai
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help