Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1438581 > unrolled thread

[PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys Dave Hansen <dave@sr71.net> - 2016-07-07 14:50 +0200
    [PATCH 8/9] pkeys: add details of system call use to Documentation/ Dave Hansen <dave@sr71.net> - 2016-07-07 14:50 +0200
    [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault bit Dave Hansen <dave@sr71.net> - 2016-07-07 15:00 +0200
      Re: [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault  bit Mel Gorman <mgorman@techsingularity.net> - 2016-07-07 16:50 +0200
        Re: [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault  bit Dave Hansen <dave@sr71.net> - 2016-07-07 17:50 +0200
    Re: [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory  Protection Keys Mel Gorman <mgorman@techsingularity.net> - 2016-07-07 16:50 +0200
    Re: [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory  Protection Keys Hugh Dickins <hughd@google.com> - 2016-07-08 20:40 +0200

#1438581 — [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys

I'm resending these because Ingo has said that he'd "love to have
some high level MM review & ack for these syscall ABI extensions."
The only changes to the code in months have been in the selftests.
So, if anyone has been putting off taking a look at these, I'd
appreciate a look now.

I also feel compelled to mention this, since I haven't before and
it gives me confidence that these interfaces are good enough:

Among other things, this feature was designed to help fix a class
of bugs in long-running applications where data corruption is
detected long after it occurs.  Today, applications either live
with the corruption, or eat a huge performance penalty from
calling mprotect() frequently.  The developers of these
applications are already running *this* *code* and are very eager
to see this feature merged and picked up in future distributions
where their customers can use it.

Other than this message, a good place to start with a review
is in the pkey(7) manpage, which I've published in HTML form here:

	https://www.sr71.net/~dave/intel/manpages/

--

Memory Protection Keys for User pages (pkeys) is a CPU feature
which will first appear on Skylake Servers, but will also be
supported on future non-server parts.  It provides a mechanism
for enforcing page-based protections, but without requiring
modification of the page tables when an application changes
wishes to change permissions.

Among other things, this feature was designed to help fix a class
of bugs in long-running applications where data corruption is
detected long after it occurs.  Applications today either live
with the corruption, or eat a huge performance penalty from
calling mprotect() frequently.  The developers of these
applications are already running this code and are very eager to
see this feature merged and picked up in future distributions
where their customers can use it.

Patches to implement execute-only mapping support using pkeys
were merged in to 4.6.  But, to do anything else useful with
pkeys, an application needs to be able to set the pkey field in
the PTE (obviously has to be done in-kernel) and make changes to
the "rights" register (using unprivileged instructions).

An application also needs to have an an allocator for the keys
themselves.  If two different parts of an application both want
to protect their data with pkeys, they first need to know which
key to use for their individual purposes.

This set introduces 5 system calls, in 3 logical groups:

1. PTE pkey setting (sys_pkey_mprotect(), patches #1-3)
2. Key allocation (sys_pkey_alloc() / sys_pkey_free(), patch #4)
3. Rights register manipulation (sys_pkey_set/get(), patch #5)

I have manpages written for some of these syscalls, and have
had multiple rounds of reviews on the manpages list.

This set is also available here:

	git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-pkeys.git pkeys-v040

I've written a set of unit tests for these interfaces, which is
available as the last patch in the series and integrated in to
kselftests.

=== diffstat ===

Dave Hansen (9):
      x86, pkeys: add fault handling for PF_PK page fault bit
      mm: implement new pkey_mprotect() system call
      x86, pkeys: make mprotect_key() mask off additional vm_flags
      x86: wire up mprotect_key() system call
      x86, pkeys: allocation/free syscalls
      x86, pkeys: add pkey set/get syscalls
      generic syscalls: wire up memory protection keys syscalls
      pkeys: add details of system call use to Documentation/
      x86, pkeys: add self-tests

 Documentation/x86/protection-keys.txt         |   63 +
 arch/alpha/include/uapi/asm/mman.h            |    5 +
 arch/mips/include/uapi/asm/mman.h             |    5 +
 arch/parisc/include/uapi/asm/mman.h           |    5 +
 arch/x86/entry/syscalls/syscall_32.tbl        |    5 +
 arch/x86/entry/syscalls/syscall_64.tbl        |    5 +
 arch/x86/include/asm/mmu.h                    |    8 +
 arch/x86/include/asm/mmu_context.h            |   25 +-
 arch/x86/include/asm/pgtable.h                |   13 +-
 arch/x86/include/asm/pgtable_64.h             |   26 +-
 arch/x86/include/asm/pgtable_types.h          |    6 -
 arch/x86/include/asm/pkeys.h                  |   80 +-
 arch/x86/kernel/fpu/xstate.c                  |   73 +-
 arch/x86/mm/fault.c                           |    9 +
 arch/x86/mm/pkeys.c                           |   38 +-
 arch/xtensa/include/uapi/asm/mman.h           |    5 +
 include/linux/pkeys.h                         |   39 +-
 include/linux/syscalls.h                      |    8 +
 include/uapi/asm-generic/mman-common.h        |    5 +
 include/uapi/asm-generic/unistd.h             |   12 +-
 mm/mprotect.c                                 |  134 +-
 tools/testing/selftests/x86/Makefile          |    3 +-
 tools/testing/selftests/x86/pkey-helpers.h    |  191 +++
 tools/testing/selftests/x86/protection_keys.c | 1316 +++++++++++++++++
 24 files changed, 2012 insertions(+), 67 deletions(-)

=== changelog ===

Changes from v3:
 * added generic syscalls declarations to include/linux/syscalls.h
   to fix arm64 compile issue.

Changes from v2:
 * selftest updates:
  * formatting changes like what Ingo asked for with MPX
  * actually call WRPKRU in __wrpkru()
  * once __wrpkru() was fixed, revealed a bug in the ptrace
    test where we were testing against the wrong pointer during
    the "baseline" test
 * Man-pages that match this set are here:
 	 http://marc.info/?l=linux-man&m=146540723525616&w=2

Changes from v1:
 * updates to alloc/free patch description calling out that
   "in-use" pkeys may still be pkey_free()'d successfully.
 * Fixed a bug in the selftest where the 'flags' argument was
   not passed to pkey_get().
 * Added all syscalls to generic syscalls header
 * Added extra checking to selftests so it doesn't fall over
   when 1G pages are made the hugetlbfs default.

Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Cc: torvalds@linux-foundation.org
Cc: akpm@linux-foundation.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: mgorman@techsingularity.net
Cc: hughd@google.com
Cc: viro@zeniv.linux.org.uk

[toc] | [next] | [standalone]

#1438587 — [PATCH 8/9] pkeys: add details of system call use to Documentation/

From: Dave Hansen <dave.hansen@linux.intel.com>

This spells out all of the pkey-related system calls that we have
and provides some example code fragments to demonstrate how we
expect them to be used.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Cc: torvalds@linux-foundation.org
Cc: akpm@linux-foundation.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: mgorman@techsingularity.net
Cc: hughd@google.com
Cc: viro@zeniv.linux.org.uk
---

 b/Documentation/x86/protection-keys.txt |   63 ++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff -puN Documentation/x86/protection-keys.txt~pkeys-120-syscall-docs Documentation/x86/protection-keys.txt
--- a/Documentation/x86/protection-keys.txt~pkeys-120-syscall-docs	2016-07-07 05:47:03.208911498 -0700
+++ b/Documentation/x86/protection-keys.txt	2016-07-07 05:47:03.212911679 -0700
@@ -18,6 +18,69 @@ even though there is theoretically space
 permissions are enforced on data access only and have no effect on
 instruction fetches.
 
+=========================== Syscalls ===========================
+
+There are 5 system calls which directly interact with pkeys:
+
+	int pkey_alloc(unsigned long flags, unsigned long init_access_rights)
+	int pkey_free(int pkey);
+	int pkey_mprotect(unsigned long start, size_t len,
+			  unsigned long prot, int pkey);
+	unsigned long pkey_get(int pkey);
+	int pkey_set(int pkey, unsigned long access_rights);
+
+Before a pkey can be used, it must first be allocated with
+pkey_alloc().  An application may either call pkey_set() or the
+WRPKRU instruction directly in order to change access permissions
+to memory covered with a key.
+
+	int real_prot = PROT_READ|PROT_WRITE;
+	pkey = pkey_alloc(0, PKEY_DENY_WRITE);
+	ptr = mmap(NULL, PAGE_SIZE, PROT_NONE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+	ret = pkey_mprotect(ptr, PAGE_SIZE, real_prot, pkey);
+	... application runs here
+
+Now, if the application needs to update the data at 'ptr', it can
+gain access, do the update, then remove its write access:
+
+	pkey_set(pkey, 0); // clear PKEY_DENY_WRITE
+	*ptr = foo; // assign something
+	pkey_set(pkey, PKEY_DENY_WRITE); // set PKEY_DENY_WRITE again
+
+Now when it frees the memory, it will also free the pkey since it
+is no longer in use:
+
+	munmap(ptr, PAGE_SIZE);
+	pkey_free(pkey);
+
+=========================== Behavior ===========================
+
+The kernel attempts to make protection keys consistent with the
+behavior of a plain mprotect().  For instance if you do this:
+
+	mprotect(ptr, size, PROT_NONE);
+	something(ptr);
+
+you can expect the same effects with protection keys when doing this:
+
+	pkey = pkey_alloc(0, PKEY_DISABLE_WRITE | PKEY_DISABLE_READ);
+	pkey_mprotect(ptr, size, PROT_READ|PROT_WRITE, pkey);
+	something(ptr);
+
+That should be true whether something() is a direct access to 'ptr'
+like:
+
+	*ptr = foo;
+
+or when the kernel does the access on the application's behalf like
+with a read():
+
+	read(fd, ptr, 1);
+
+The kernel will send a SIGSEGV in both cases, but si_code will be set
+to SEGV_PKERR when violating protection keys versus SEGV_ACCERR when
+the plain mprotect() permissions are violated.
+
 =========================== Config Option ===========================
 
 This config option adds approximately 1.5kb of text. and 50 bytes of
_

[toc] | [prev] | [next] | [standalone]

#1438595 — [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault bit

From: Dave Hansen <dave.hansen@linux.intel.com>

PF_PK means that a memory access violated the protection key
access restrictions.  It is unconditionally an access_error()
because the permissions set on the VMA don't matter (the PKRU
value overrides it), and we never "resolve" PK faults (like
how a COW can "resolve write fault).

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: x86@kernel.org
Cc: torvalds@linux-foundation.org
Cc: akpm@linux-foundation.org
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: mgorman@techsingularity.net
Cc: hughd@google.com
Cc: viro@zeniv.linux.org.uk
---

 b/arch/x86/mm/fault.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff -puN arch/x86/mm/fault.c~pkeys-105-add-pk-to-fault arch/x86/mm/fault.c
--- a/arch/x86/mm/fault.c~pkeys-105-add-pk-to-fault	2016-07-07 05:46:59.554745699 -0700
+++ b/arch/x86/mm/fault.c	2016-07-07 05:46:59.558745881 -0700
@@ -1112,6 +1112,15 @@ access_error(unsigned long error_code, s
 {
 	/* This is only called for the current mm, so: */
 	bool foreign = false;
+
+	/*
+	 * Read or write was blocked by protection keys.  This is
+	 * always an unconditional error and can never result in
+	 * a follow-up action to resolve the fault, like a COW.
+	 */
+	if (error_code & PF_PK)
+		return 1;
+
 	/*
 	 * Make sure to check the VMA so that we do not perform
 	 * faults just to hit a PF_PK as soon as we fill in a
_

[toc] | [prev] | [next] | [standalone]

#1438657 — Re: [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault bit

On Thu, Jul 07, 2016 at 05:47:20AM -0700, Dave Hansen wrote:
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> PF_PK means that a memory access violated the protection key
> access restrictions.  It is unconditionally an access_error()
> because the permissions set on the VMA don't matter (the PKRU
> value overrides it), and we never "resolve" PK faults (like
> how a COW can "resolve write fault).
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>

An access fault gets propgated as SEGV_PKUERR. What happens if glibc
does not recognise it?

-- 
Mel Gorman
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1438691 — Re: [PATCH 1/9] x86, pkeys: add fault handling for PF_PK page fault bit

On 07/07/2016 07:40 AM, Mel Gorman wrote:
> On Thu, Jul 07, 2016 at 05:47:20AM -0700, Dave Hansen wrote:
>> From: Dave Hansen <dave.hansen@linux.intel.com>
>> PF_PK means that a memory access violated the protection key
>> access restrictions.  It is unconditionally an access_error()
>> because the permissions set on the VMA don't matter (the PKRU
>> value overrides it), and we never "resolve" PK faults (like
>> how a COW can "resolve write fault).
>>
>> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> 
> An access fault gets propgated as SEGV_PKUERR. What happens if glibc
> does not recognise it?

It passes it through to the handler without any side-effects.  I don't
think it does anything differently with SEGV_* codes that it knows about
vs. unknown ones.  The only negative side-effect that I can think of is
that it won't have a nice error message for it.

[toc] | [prev] | [next] | [standalone]

#1438652 — Re: [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys

On Thu, Jul 07, 2016 at 05:47:19AM -0700, Dave Hansen wrote:
> I'm resending these because Ingo has said that he'd "love to have
> some high level MM review & ack for these syscall ABI extensions."
> The only changes to the code in months have been in the selftests.
> So, if anyone has been putting off taking a look at these, I'd
> appreciate a look now.
> 

I took a look at the patches other than the self-tests and the
documentation. I did not see any major problems other than thinking
there is a lot of ways for userspace to shoot itself in the foot and
lose the protection pkeys is meant to give.

That said, consider the review to be unreliable on the grounds I haven't
followed pkeys development and this is my first time looking in its
general direction.

-- 
Mel Gorman
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1439739 — Re: [PATCH 0/9] [REVIEW-REQUEST] [v4] System Calls for Memory Protection Keys

On Thu, 7 Jul 2016, Dave Hansen wrote:

> I'm resending these because Ingo has said that he'd "love to have
> some high level MM review & ack for these syscall ABI extensions."
> The only changes to the code in months have been in the selftests.
> So, if anyone has been putting off taking a look at these, I'd
> appreciate a look now.

Please expect nothing from me.  I've paid no attention to memory
protection keys, and they're way off my radar of interest.  Sorry,
if I spend my time looking at stuff I have little to contribute to,
then I'm left with no time at all for what I should really be doing.

Hugh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web