x86, pkeys: two protection keys bug fixes
Subsystems (5)
Dave Hansen
maintainerv4 cover x86, pkeys: two protection keys bug fixes
Hi x86 maintainers,
This set has been seen quite a few changes and additions since the last post. Details below.
Changes from v3:
* Reordered patches following Ingo's recommendations: Introduce failing selftests first, then the kernel code to fix the test failure.
* Increase verbosity and accuracy of do_not_expect_pk_fault() messages.
* Removed abort() use from tests. Crashing is not nice.
* Remove some dead debugging code, fixing dprint_in_signal.
* Fix deadlocks from using printf() and friends in signal handlers.
Changes from v2:
* Clarified commit message in patch 1/9 taking some feedback from Shuah.
Changes from v1:
* Added Fixes: and cc'd stable. No code changes.
--
This fixes two bugs, and adds selftests to make sure they stay fixed:
1. pkey 0 was not usable via mprotect_pkey() because it had never been explicitly allocated.
2. mprotect(PROT_EXEC) memory could sometimes be left with the implicit exec-only protection key assigned.
I already posted #1 previously. I'm including them both here because I don't think it's been picked up in case folks want to pull these all in a single bundle.
Dave Hansen (13):
x86/pkeys/selftests: give better unexpected fault error messages x86/pkeys/selftests: Stop using assert() x86/pkeys/selftests: remove dead debugging code, fix dprint_in_signal x86/pkeys/selftests: avoid printf-in-signal deadlocks x86/pkeys/selftests: Allow faults on unknown keys x86/pkeys/selftests: Factor out "instruction page" x86/pkeys/selftests: Add PROT_EXEC test x86/pkeys/selftests: Fix pkey exhaustion test off-by-one x86/pkeys: Override pkey when moving away from PROT_EXEC x86/pkeys/selftests: Fix pointer math x86/pkeys/selftests: Save off 'prot' for allocations x86/pkeys/selftests: Add a test for pkey 0
x86/pkeys: Do not special case protection key 0
arch/x86/include/asm/mmu_context.h | 2 +- arch/x86/include/asm/pkeys.h | 18 +- arch/x86/mm/pkeys.c | 21 +- tools/testing/selftests/x86/pkey-helpers.h | 20 +- tools/testing/selftests/x86/protection_keys.c | 187 +++++++++++++----- 5 files changed, 173 insertions(+), 75 deletions(-)
Dave Hansen
maintainerPATCH v4 1/13 x86/pkeys/selftests: Give better unexpected fault error messages
From: Dave Hansen <[email protected]>
do_not_expect_pk_fault() is a helper that we call when we do not expect a PK fault to have occurred. But, it is a function, which means that it obscures the line numbers from pkey_assert(). It also gives no details.
Replace it with an implementation that gives nice line numbers and also lets callers pass in a more descriptive message about what happened that caused the unexpected fault.
---
b/tools/testing/selftests/x86/protection_keys.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-give-better-pkey-fault-errors +7 -6
@@ -939,10 +939,11 @@ void expected_pk_fault(int pkey) last_si_pkey =-1 ; } -void do_not_expect_pk_fault (void ) -{ -pkey_assert (last_pkru_faults == pkru_faults); -} +#define do_not_expect_pk_fault (msg) do { \ + if (last_pkru_faults != pkru_faults) \ + dprintf0("unexpected PK fault: %s\n", msg); \ + pkey_assert(last_pkru_faults == pkru_faults); \ +} while (0)int test_fds[10 ] = {-1 };int nr_test_fds;
@@ -1228,7 +1229,7 @@ void test_ptrace_of_child(int *ptr, u16pkey_assert (ret !=-1 );/* Now access from the current task, and expect NO exception: */ peek_result =read_ptr (plain_ptr); -do_not_expect_pk_fault (); +do_not_expect_pk_fault ("read plain pointer after ptrace" ); ret =ptrace (PTRACE_DETACH , child_pid, ignored,0 );pkey_assert (ret !=-1 );
@@ -1272,7 +1273,7 @@ void test_executing_on_unreadable_memory*/ madvise (p1,PAGE_SIZE ,MADV_DONTNEED );lots_o_noops_around_write (&scratch); -do_not_expect_pk_fault (); +do_not_expect_pk_fault ("executing on PROT_EXEC memory" ); ptr_contents =read_ptr (p1);dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents);expected_pk_fault (pkey);
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Give better unexpected fault error messages
x86/pkeys/selftests: Give better unexpected fault error messages
Commit-ID: 55556b0b2016806b2e16a20b62d143383983a34a Gitweb: https://git.kernel.org/tip/55556b0b2016806b2e16a20b62d143383983a34a Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:38 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Give better unexpected fault error messages
do_not_expect_pk_fault() is a helper that we call when we do not expect a PK fault to have occurred. But, it is a function, which means that it obscures the line numbers from pkey_assert(). It also gives no details.
Replace it with an implementation that gives nice line numbers and also lets callers pass in a more descriptive message about what happened that caused the unexpected fault.
--- tools/testing/selftests/x86/protection_keys.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
tools/testing/selftests/x86/protection_keys.c +7 -6
@@ -954,10 +954,11 @@ void expected_pk_fault(int pkey) last_si_pkey =-1 ; } -void do_not_expect_pk_fault (void ) -{ -pkey_assert (last_pkru_faults == pkru_faults); -} +#define do_not_expect_pk_fault (msg) do { \ + if (last_pkru_faults != pkru_faults) \ + dprintf0("unexpected PK fault: %s\n", msg); \ + pkey_assert(last_pkru_faults == pkru_faults); \ +} while (0)int test_fds[10 ] = {-1 };int nr_test_fds;
@@ -1243,7 +1244,7 @@ void test_ptrace_of_child(int *ptr, u16 pkey)pkey_assert (ret !=-1 );/* Now access from the current task, and expect NO exception: */ peek_result =read_ptr (plain_ptr); -do_not_expect_pk_fault (); +do_not_expect_pk_fault ("read plain pointer after ptrace" ); ret =ptrace (PTRACE_DETACH , child_pid, ignored,0 );pkey_assert (ret !=-1 );
@@ -1287,7 +1288,7 @@ void test_executing_on_unreadable_memory(int *ptr, u16 pkey)*/ madvise (p1,PAGE_SIZE ,MADV_DONTNEED );lots_o_noops_around_write (&scratch); -do_not_expect_pk_fault (); +do_not_expect_pk_fault ("executing on PROT_EXEC memory" ); ptr_contents =read_ptr (p1);dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents);expected_pk_fault (pkey);
Dave Hansen
maintainerPATCH v4 2/13 x86/pkeys/selftests: Stop using assert()
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
If we use assert(), the program "crashes". That can be scary to users, so stop doing it. Just exit with a >0 exit code instead.
---
b/tools/testing/selftests/x86/protection_keys.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-do-not-assert +8 -4
@@ -72,10 +72,9 @@ extern void abort_hooks(void); test_nr, iteration_nr); \dprintf0 ("errno at assert: %d" , errno); \abort_hooks (); \ -assert (condition); \ +exit (__LINE__); \ } \ }while (0 ) -#define raw_assert (cond) assert(cond) void cat_into_file(char *str, char *file) {
@@ -87,12 +86,17 @@ void cat_into_file(char *str, char *file* these need to be raw because they are called under * pkey_assert() */ -raw_assert (fd >=0 ); +if (fd <0 ) { +fprintf (stderr,"error opening '%s'\n" , str); +perror ("error: " ); +exit (__LINE__); + } + ret =write (fd, str,strlen (str));if (ret !=strlen (str)) {perror ("write to file failed" );fprintf (stderr,"filename: '%s' str: '%s'\n" , file, str); -raw_assert (0 ); +exit (__LINE__); }close (fd); }
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Stop using assert()
x86/pkeys/selftests: Stop using assert()
Commit-ID: 86b9eea230edf4c67d4d4a70fba9b74505867a25 Gitweb: https://git.kernel.org/tip/86b9eea230edf4c67d4d4a70fba9b74505867a25 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:40 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Stop using assert()
If we use assert(), the program "crashes". That can be scary to users, so stop doing it. Just exit with a >0 exit code instead.
--- tools/testing/selftests/x86/protection_keys.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
tools/testing/selftests/x86/protection_keys.c +8 -4
@@ -72,10 +72,9 @@ extern void abort_hooks(void); test_nr, iteration_nr); \dprintf0 ("errno at assert: %d" , errno); \abort_hooks (); \ -assert (condition); \ +exit (__LINE__); \ } \ }while (0 ) -#define raw_assert (cond) assert(cond) void cat_into_file(char *str, char *file) {
@@ -87,12 +86,17 @@ void cat_into_file(char *str, char *file)* these need to be raw because they are called under * pkey_assert() */ -raw_assert (fd >=0 ); +if (fd <0 ) { +fprintf (stderr,"error opening '%s'\n" , str); +perror ("error: " ); +exit (__LINE__); + } + ret =write (fd, str,strlen (str));if (ret !=strlen (str)) {perror ("write to file failed" );fprintf (stderr,"filename: '%s' str: '%s'\n" , file, str); -raw_assert (0 ); +exit (__LINE__); }close (fd); }
Dave Hansen
maintainerPATCH v4 3/13 x86/pkeys/selftests: Remove dead debugging code, fix dprint_in_signal
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
There is some noisy debug code at the end of the signal handler. It was disabled by an early, unconditional "return". However, that return also hid a dprint_in_signal=0, which kept dprint_in_signal=1 and effectively locked us into permanent dprint_in_signal=1 behavior.
Remove the return and the dead code, fixing dprint_in_signal.
---
b/tools/testing/selftests/x86/protection_keys.c | 16 ---------------- 1 file changed, 16 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-remove-dead-code-after-return +0 -16
@@ -315,22 +315,6 @@ void signal_handler(int signum, siginfo_dprintf1 ("WARNING: set PRKU=0 to allow faulting instruction to continue\n" ); pkru_faults++;dprintf1 ("<<<<==================================================\n" ); -return ; -if (trapno ==14 ) { -fprintf (stderr, -"ERROR: In signal handler, page fault, trapno = %d, ip = %016lx\n" , - trapno, ip); -fprintf (stderr,"si_addr %p\n" , si->si_addr); -fprintf (stderr,"REG_ERR: %lx\n" , - (unsigned long )uctxt->uc_mcontext.gregs[REG_ERR ]); -exit (1 ); - }else { -fprintf (stderr,"unexpected trap %d! at 0x%lx\n" , trapno, ip); -fprintf (stderr,"si_addr %p\n" , si->si_addr); -fprintf (stderr,"REG_ERR: %lx\n" , - (unsigned long )uctxt->uc_mcontext.gregs[REG_ERR ]); -exit (2 ); - } dprint_in_signal =0 ; }
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Remove dead debugging code, fix dprint_in_signal
x86/pkeys/selftests: Remove dead debugging code, fix dprint_in_signal
Commit-ID: a50093d60464dd51d1ae0c2267b0abe9e1de77f3 Gitweb: https://git.kernel.org/tip/a50093d60464dd51d1ae0c2267b0abe9e1de77f3 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:42 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Remove dead debugging code, fix dprint_in_signal
There is some noisy debug code at the end of the signal handler. It was disabled by an early, unconditional "return". However, that return also hid a dprint_in_signal=0, which kept dprint_in_signal=1 and effectively locked us into permanent dprint_in_signal=1 behavior.
Remove the return and the dead code, fixing dprint_in_signal.
--- tools/testing/selftests/x86/protection_keys.c | 16 ---------------- 1 file changed, 16 deletions(-)
tools/testing/selftests/x86/protection_keys.c +0 -16
@@ -325,22 +325,6 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)dprintf1 ("WARNING: set PRKU=0 to allow faulting instruction to continue\n" ); pkru_faults++;dprintf1 ("<<<<==================================================\n" ); -return ; -if (trapno ==14 ) { -fprintf (stderr, -"ERROR: In signal handler, page fault, trapno = %d, ip = %016lx\n" , - trapno, ip); -fprintf (stderr,"si_addr %p\n" , si->si_addr); -fprintf (stderr,"REG_ERR: %lx\n" , - (unsigned long )uctxt->uc_mcontext.gregs[REG_ERR ]); -exit (1 ); - }else { -fprintf (stderr,"unexpected trap %d! at 0x%lx\n" , trapno, ip); -fprintf (stderr,"si_addr %p\n" , si->si_addr); -fprintf (stderr,"REG_ERR: %lx\n" , - (unsigned long )uctxt->uc_mcontext.gregs[REG_ERR ]); -exit (2 ); - } dprint_in_signal =0 ; }
Dave Hansen
maintainerPATCH v4 4/13 x86/pkeys/selftests: Avoid printf-in-signal deadlocks
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
printf() and friends are unusable in signal handlers. They deadlock. The pkey selftest does not do any normal printing in signal handlers, only extra debugging. So, just print the format string so we get *some* output when debugging.
---
b/tools/testing/selftests/x86/pkey-helpers.h | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
tools/testing/selftests/x86/pkey-helpers.h renamed from tools/testing/selftests/x86/pkey-helpers.h~pkeys-selftests-avoid-print-in-signal-deadlocks +8 -12
@@ -26,30 +26,26 @@ static inline void sigsafe_printf(const {va_list ap; -va_start (ap, format);if (!dprint_in_signal) { +va_start (ap, format);vprintf (format, ap); +va_end (ap); }else {int ret; -int len =vsnprintf (dprint_in_signal_buffer, -DPRINT_IN_SIGNAL_BUF_SIZE , - format, ap);/* -* len is amount that would have been printed, -* but actual write is truncated at BUF_SIZE. +* No printf() functions are signal-safe. +* They deadlock easily. Write the format +* string to get some output, even if +* incomplete. */ -if (len >DPRINT_IN_SIGNAL_BUF_SIZE ) - len =DPRINT_IN_SIGNAL_BUF_SIZE ; - ret =write (1 , dprint_in_signal_buffer, len); + ret =write (1 , format,strlen (format));if (ret <0 ) -abort (); +exit (1 ); } -va_end (ap); } #define dprintf_level(level, args...) do { \ if (level <= DEBUG_LEVEL) \ sigsafe_printf(args); \ - fflush(NULL); \ } while (0) #define dprintf0(args...) dprintf_level(0, args) #define dprintf1(args...) dprintf_level(1, args)
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Avoid printf-in-signal deadlocks
x86/pkeys/selftests: Avoid printf-in-signal deadlocks
Commit-ID: caf9eb6b4c82fc6cbd03697052ff22d97b0c377b Gitweb: https://git.kernel.org/tip/caf9eb6b4c82fc6cbd03697052ff22d97b0c377b Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:44 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Avoid printf-in-signal deadlocks
printf() and friends are unusable in signal handlers. They deadlock. The pkey selftest does not do any normal printing in signal handlers, only extra debugging. So, just print the format string so we get *some* output when debugging.
--- tools/testing/selftests/x86/pkey-helpers.h | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
tools/testing/selftests/x86/pkey-helpers.h +8 -12
@@ -26,30 +26,26 @@ static inline void sigsafe_printf(const char *format, ...) {va_list ap; -va_start (ap, format);if (!dprint_in_signal) { +va_start (ap, format);vprintf (format, ap); +va_end (ap); }else {int ret; -int len =vsnprintf (dprint_in_signal_buffer, -DPRINT_IN_SIGNAL_BUF_SIZE , - format, ap);/* -* len is amount that would have been printed, -* but actual write is truncated at BUF_SIZE. +* No printf() functions are signal-safe. +* They deadlock easily. Write the format +* string to get some output, even if +* incomplete. */ -if (len >DPRINT_IN_SIGNAL_BUF_SIZE ) - len =DPRINT_IN_SIGNAL_BUF_SIZE ; - ret =write (1 , dprint_in_signal_buffer, len); + ret =write (1 , format,strlen (format));if (ret <0 ) -abort (); +exit (1 ); } -va_end (ap); } #define dprintf_level(level, args...) do { \ if (level <= DEBUG_LEVEL) \ sigsafe_printf(args); \ - fflush(NULL); \ } while (0) #define dprintf0(args...) dprintf_level(0, args) #define dprintf1(args...) dprintf_level(1, args)
Dave Hansen
maintainerPATCH v4 5/13 x86/pkeys/selftests: Allow faults on unknown keys
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
The exec-only pkey is allocated inside the kernel and userspace is not told what it is. So, allow PK faults to occur that have an unknown key.
---
b/tools/testing/selftests/x86/protection_keys.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-unknown-exec-only-key +9 -1
@@ -906,13 +906,21 @@ void *malloc_pkey(long size, int prot, u }int last_pkru_faults; +#define UNKNOWN_PKEY -2void expected_pk_fault (int pkey) {dprintf2 ("%s(): last_pkru_faults: %d pkru_faults: %d\n" , __func__, last_pkru_faults, pkru_faults);dprintf2 ("%s(%d): last_si_pkey: %d\n" , __func__, pkey, last_si_pkey);pkey_assert (last_pkru_faults +1 == pkru_faults); -pkey_assert (last_si_pkey == pkey); + +/* +* For exec-only memory, we do not know the pkey in +* advance, so skip this check. +*/ +if (pkey !=UNKNOWN_PKEY ) +pkey_assert (last_si_pkey == pkey); +/* * The signal handler shold have cleared out PKRU to let the * test program continue. We now have to restore it.
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Allow faults on unknown keys
x86/pkeys/selftests: Allow faults on unknown keys
Commit-ID: 7e7fd67ca39335a49619729821efb7cbdd674eb0 Gitweb: https://git.kernel.org/tip/7e7fd67ca39335a49619729821efb7cbdd674eb0 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:46 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Allow faults on unknown keys
The exec-only pkey is allocated inside the kernel and userspace is not told what it is. So, allow PK faults to occur that have an unknown key.
--- tools/testing/selftests/x86/protection_keys.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/protection_keys.c +9 -1
@@ -921,13 +921,21 @@ void *malloc_pkey(long size, int prot, u16 pkey) }int last_pkru_faults; +#define UNKNOWN_PKEY -2void expected_pk_fault (int pkey) {dprintf2 ("%s(): last_pkru_faults: %d pkru_faults: %d\n" , __func__, last_pkru_faults, pkru_faults);dprintf2 ("%s(%d): last_si_pkey: %d\n" , __func__, pkey, last_si_pkey);pkey_assert (last_pkru_faults +1 == pkru_faults); -pkey_assert (last_si_pkey == pkey); + +/* +* For exec-only memory, we do not know the pkey in +* advance, so skip this check. +*/ +if (pkey !=UNKNOWN_PKEY ) +pkey_assert (last_si_pkey == pkey); +/* * The signal handler shold have cleared out PKRU to let the * test program continue. We now have to restore it.
Dave Hansen
maintainerPATCH v4 6/13 x86/pkeys/selftests: Factor out "instruction page"
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
We currently have an execute-only test, but it is for the explicit mprotect_pkey() interface. We will soon add a test for the implicit mprotect(PROT_EXEC) enterface. We need this code in both tests.
---
b/tools/testing/selftests/x86/protection_keys.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-get_pointer_to_instructions +17 -4
@@ -1238,12 +1238,9 @@ void test_ptrace_of_child(int *ptr, u16free (plain_ptr_unaligned); } -void test_executing_on_unreadable_memory (int *ptr,u16 pkey) +void *get_pointer_to_instructions (void ) {void *p1; -int scratch; -int ptr_contents; -int ret; p1 =ALIGN_PTR_UP (&lots_o_noops_around_write,PAGE_SIZE );dprintf3 ("&lots_o_noops: %p\n" , &lots_o_noops_around_write);
@@ -1253,7 +1250,23 @@ void test_executing_on_unreadable_memory/* Point 'p1' at the *second* page of the function: */ p1 +=PAGE_SIZE ; +/* +* Try to ensure we fault this in on next touch to ensure +* we get an instruction fault as opposed to a data one +*/ madvise (p1,PAGE_SIZE ,MADV_DONTNEED ); + +return p1; +} + +void test_executing_on_unreadable_memory (int *ptr,u16 pkey) +{ +void *p1; +int scratch; +int ptr_contents; +int ret; + + p1 =get_pointer_to_instructions ();lots_o_noops_around_write (&scratch); ptr_contents =read_ptr (p1);dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents);
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Factor out "instruction page"
x86/pkeys/selftests: Factor out "instruction page"
Commit-ID: 3fcd2b2d928904cbf30b01e2c5e4f1dd2f9ab262 Gitweb: https://git.kernel.org/tip/3fcd2b2d928904cbf30b01e2c5e4f1dd2f9ab262 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:47 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Factor out "instruction page"
We currently have an execute-only test, but it is for the explicit mprotect_pkey() interface. We will soon add a test for the implicit mprotect(PROT_EXEC) enterface. We need this code in both tests.
--- tools/testing/selftests/x86/protection_keys.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
tools/testing/selftests/x86/protection_keys.c +17 -4
@@ -1253,12 +1253,9 @@ void test_ptrace_of_child(int *ptr, u16 pkey)free (plain_ptr_unaligned); } -void test_executing_on_unreadable_memory (int *ptr,u16 pkey) +void *get_pointer_to_instructions (void ) {void *p1; -int scratch; -int ptr_contents; -int ret; p1 =ALIGN_PTR_UP (&lots_o_noops_around_write,PAGE_SIZE );dprintf3 ("&lots_o_noops: %p\n" , &lots_o_noops_around_write);
@@ -1268,7 +1265,23 @@ void test_executing_on_unreadable_memory(int *ptr, u16 pkey)/* Point 'p1' at the *second* page of the function: */ p1 +=PAGE_SIZE ; +/* +* Try to ensure we fault this in on next touch to ensure +* we get an instruction fault as opposed to a data one +*/ madvise (p1,PAGE_SIZE ,MADV_DONTNEED ); + +return p1; +} + +void test_executing_on_unreadable_memory (int *ptr,u16 pkey) +{ +void *p1; +int scratch; +int ptr_contents; +int ret; + + p1 =get_pointer_to_instructions ();lots_o_noops_around_write (&scratch); ptr_contents =read_ptr (p1);dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents);
Dave Hansen
maintainerPATCH v4 7/13 x86/pkeys/selftests: Add PROT_EXEC test
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
Under the covers, implement executable-only memory with protection keys when userspace calls mprotect(PROT_EXEC).
But, we did not have a selftest for that. Now we do.
---
b/tools/testing/selftests/x86/protection_keys.c | 44 ++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-prot_exec +44 -0
@@ -1288,6 +1288,49 @@ void test_executing_on_unreadable_memoryexpected_pk_fault (pkey); } +void test_implicit_mprotect_exec_only_memory (int *ptr,u16 pkey) +{ +void *p1; +int scratch; +int ptr_contents; +int ret; + +dprintf1 ("%s() start\n" , __func__); + + p1 =get_pointer_to_instructions (); +lots_o_noops_around_write (&scratch); + ptr_contents =read_ptr (p1); +dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); + +/* Use a *normal* mprotect(), not mprotect_pkey(): */ + ret =mprotect (p1,PAGE_SIZE ,PROT_EXEC ); +pkey_assert (!ret); + +dprintf2 ("pkru: %x\n" ,rdpkru ()); + +/* Make sure this is an *instruction* fault */ +madvise (p1,PAGE_SIZE ,MADV_DONTNEED ); +lots_o_noops_around_write (&scratch); +do_not_expect_pk_fault ("executing on PROT_EXEC memory" ); + ptr_contents =read_ptr (p1); +dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); +expected_pk_fault (UNKNOWN_PKEY ); + +/* +* Put the memory back to non-PROT_EXEC. Should clear the +* exec-only pkey off the VMA and allow it to be readable +* again. Go to PROT_NONE first to check for a kernel bug +* that did not clear the pkey when doing PROT_NONE. +*/ + ret =mprotect (p1,PAGE_SIZE ,PROT_NONE ); +pkey_assert (!ret); + + ret =mprotect (p1,PAGE_SIZE ,PROT_READ |PROT_EXEC ); +pkey_assert (!ret); + ptr_contents =read_ptr (p1); +do_not_expect_pk_fault ("plain read on recently PROT_EXEC area" ); +} +void test_mprotect_pkey_on_unsupported_cpu (int *ptr,u16 pkey) {int size =PAGE_SIZE ;
@@ -1312,6 +1355,7 @@ void (*pkey_tests[])(int *ptr, u16 pkey) test_kernel_gup_of_access_disabled_region, test_kernel_gup_write_to_write_disabled_region, test_executing_on_unreadable_memory, + test_implicit_mprotect_exec_only_memory, test_ptrace_of_child, test_pkey_syscalls_on_non_allocated_pkey, test_pkey_syscalls_bad_args,
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Add PROT_EXEC test
x86/pkeys/selftests: Add PROT_EXEC test
Commit-ID: 6af17cf89e99b64cf1f660bf848755442ab2f047 Gitweb: https://git.kernel.org/tip/6af17cf89e99b64cf1f660bf848755442ab2f047 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:48 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Add PROT_EXEC test
Under the covers, implement executable-only memory with protection keys when userspace calls mprotect(PROT_EXEC).
But, we did not have a selftest for that. Now we do.
--- tools/testing/selftests/x86/protection_keys.c | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
tools/testing/selftests/x86/protection_keys.c +44 -0
@@ -1303,6 +1303,49 @@ void test_executing_on_unreadable_memory(int *ptr, u16 pkey)expected_pk_fault (pkey); } +void test_implicit_mprotect_exec_only_memory (int *ptr,u16 pkey) +{ +void *p1; +int scratch; +int ptr_contents; +int ret; + +dprintf1 ("%s() start\n" , __func__); + + p1 =get_pointer_to_instructions (); +lots_o_noops_around_write (&scratch); + ptr_contents =read_ptr (p1); +dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); + +/* Use a *normal* mprotect(), not mprotect_pkey(): */ + ret =mprotect (p1,PAGE_SIZE ,PROT_EXEC ); +pkey_assert (!ret); + +dprintf2 ("pkru: %x\n" ,rdpkru ()); + +/* Make sure this is an *instruction* fault */ +madvise (p1,PAGE_SIZE ,MADV_DONTNEED ); +lots_o_noops_around_write (&scratch); +do_not_expect_pk_fault ("executing on PROT_EXEC memory" ); + ptr_contents =read_ptr (p1); +dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); +expected_pk_fault (UNKNOWN_PKEY ); + +/* +* Put the memory back to non-PROT_EXEC. Should clear the +* exec-only pkey off the VMA and allow it to be readable +* again. Go to PROT_NONE first to check for a kernel bug +* that did not clear the pkey when doing PROT_NONE. +*/ + ret =mprotect (p1,PAGE_SIZE ,PROT_NONE ); +pkey_assert (!ret); + + ret =mprotect (p1,PAGE_SIZE ,PROT_READ |PROT_EXEC ); +pkey_assert (!ret); + ptr_contents =read_ptr (p1); +do_not_expect_pk_fault ("plain read on recently PROT_EXEC area" ); +} +void test_mprotect_pkey_on_unsupported_cpu (int *ptr,u16 pkey) {int size =PAGE_SIZE ;
@@ -1327,6 +1370,7 @@ void (*pkey_tests[])(int *ptr, u16 pkey) = { test_kernel_gup_of_access_disabled_region, test_kernel_gup_write_to_write_disabled_region, test_executing_on_unreadable_memory, + test_implicit_mprotect_exec_only_memory, test_ptrace_of_child, test_pkey_syscalls_on_non_allocated_pkey, test_pkey_syscalls_bad_args,
Dave Hansen
maintainerPATCH v4 8/13 x86/pkeys/selftests: Fix pkey exhaustion test off-by-one
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
In our "exhaust all pkeys" test, we make sure that there is the expected number available. Turns out that the test did not cover the execute-only key, but discussed it anyway. It did *not* discuss the test-allocated key.
Now that we have a test for the mprotect(PROT_EXEC) case, this off-by-one issue showed itself. Correct the off-by- one and add the explanation for the case we missed.
---
b/tools/testing/selftests/x86/protection_keys.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-exhaust-off-by-one +8 -5
@@ -1148,12 +1148,15 @@ void test_pkey_alloc_exhaust(int *ptr, upkey_assert (i <NR_PKEYS *2 );/* -* There are 16 pkeys supported in hardware. One is taken -* up for the default (0) and another can be taken up by -* an execute-only mapping. Ensure that we can allocate -* at least 14 (16-2). +* There are 16 pkeys supported in hardware. Three are +* allocated by the time we get here: +* 1. The default key (0) +* 2. One possibly consumed by an execute-only mapping. +* 3. One allocated by the test code and passed in via +* 'pkey' to this function. +* Ensure that we can allocate at least another 13 (16-3). */ -pkey_assert (i >=NR_PKEYS -2 ); +pkey_assert (i >=NR_PKEYS -3 );for (i =0 ; i < nr_allocated_pkeys; i++) { err =sys_pkey_free (allocated_pkeys[i]);
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Fix pkey exhaustion test off-by-one
x86/pkeys/selftests: Fix pkey exhaustion test off-by-one
Commit-ID: f50b4878329ab61d8e05796f655adeb6f5fb57c6 Gitweb: https://git.kernel.org/tip/f50b4878329ab61d8e05796f655adeb6f5fb57c6 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:50 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Fix pkey exhaustion test off-by-one
In our "exhaust all pkeys" test, we make sure that there is the expected number available. Turns out that the test did not cover the execute-only key, but discussed it anyway. It did *not* discuss the test-allocated key.
Now that we have a test for the mprotect(PROT_EXEC) case, this off-by-one issue showed itself. Correct the off-by- one and add the explanation for the case we missed.
--- tools/testing/selftests/x86/protection_keys.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c +8 -5
@@ -1163,12 +1163,15 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)pkey_assert (i <NR_PKEYS *2 );/* -* There are 16 pkeys supported in hardware. One is taken -* up for the default (0) and another can be taken up by -* an execute-only mapping. Ensure that we can allocate -* at least 14 (16-2). +* There are 16 pkeys supported in hardware. Three are +* allocated by the time we get here: +* 1. The default key (0) +* 2. One possibly consumed by an execute-only mapping. +* 3. One allocated by the test code and passed in via +* 'pkey' to this function. +* Ensure that we can allocate at least another 13 (16-3). */ -pkey_assert (i >=NR_PKEYS -2 ); +pkey_assert (i >=NR_PKEYS -3 );for (i =0 ; i < nr_allocated_pkeys; i++) { err =sys_pkey_free (allocated_pkeys[i]);
Dave Hansen
maintainerPATCH v4 9/13 x86/pkeys: Override pkey when moving away from PROT_EXEC
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
I got a bug report that the following code (roughly) was causing a SIGSEGV:
mprotect(ptr, size, PROT_EXEC); mprotect(ptr, size, PROT_NONE); mprotect(ptr, size, PROT_READ); *ptr = 100;
The problem is hit when the mprotect(PROT_EXEC)
is implicitly assigned a protection key to the VMA, and made that key ACCESS_DENY|WRITE_DENY. The PROT_NONE mprotect() failed to remove the protection key, and the PROT_NONE-> PROT_READ left the PTE usable, but the pkey still in place and left the memory inaccessible.
To fix this, we ensure that we always "override" the pkee at mprotect() if the VMA does not have execute-only permissions, but the VMA has the execute-only pkey.
We had a check for PROT_READ/WRITE, but it did not work for PROT_NONE. This entirely removes the PROT_* checks, which ensures that PROT_NONE now works.
---
b/arch/x86/include/asm/pkeys.h | 12 +++++++++++- b/arch/x86/mm/pkeys.c | 21 +++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-)
arch/x86/include/asm/pkeys.h renamed from arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively +11 -1
@@ -2,6 +2,8 @@#ifndef _ASM_X86_PKEYS_H#define _ASM_X86_PKEYS_H +#define ARCH_DEFAULT_PKEY 0 +#define arch_max_pkey () (boot_cpu_has(X86_FEATURE_OSPKE) ? 16 : 1) extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
@@ -15,7 +17,7 @@ extern int __execute_only_pkey(struct mmstatic inline int execute_only_pkey (struct mm_struct *mm) {if (!boot_cpu_has (X86_FEATURE_OSPKE )) -return 0 ; +return ARCH_DEFAULT_PKEY ;return __execute_only_pkey (mm); }
@@ -56,6 +58,14 @@ bool mm_pkey_is_allocated(struct mm_strureturn false;if (pkey >=arch_max_pkey ())return false; +/* +* The exec-only pkey is set in the allocation map, but +* is not available to any of the user interfaces like +* mprotect_pkey(). +*/ +if (pkey == mm->context.execute_only_pkey) +return false; +return mm_pkey_allocation_map (mm) & (1U << pkey); }
arch/x86/mm/pkeys.c renamed from arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively +11 -10
@@ -94,26 +94,27 @@ int __arch_override_mprotect_pkey(struct*/ if (pkey !=-1 )return pkey; -/* -* Look for a protection-key-drive execute-only mapping -* which is now being given permissions that are not -* execute-only. Move it back to the default pkey. -*/ -if (vma_is_pkey_exec_only (vma) && - (prot & (PROT_READ |PROT_WRITE ))) { -return 0 ; - } +/* * The mapping is execute-only. Go try to get the * execute-only protection key. If we fail to do that, * fall through as if we do not have execute-only -* support. +* support in this mm. */ if (prot ==PROT_EXEC ) { pkey =execute_only_pkey (vma->vm_mm);if (pkey >0 )return pkey; + }else if (vma_is_pkey_exec_only (vma)) { +/* +* Protections are *not* PROT_EXEC, but the mapping +* is using the exec-only pkey. This mapping was +* PROT_EXEC and will no longer be. Move back to +* the default pkey. +*/ +return ARCH_DEFAULT_PKEY ; } +/* * This is a vanilla, non-pkey mprotect (or we failed to * setup execute-only), inherit the pkey from the VMA we
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys: Override pkey when moving away from PROT_EXEC
x86/pkeys: Override pkey when moving away from PROT_EXEC
Commit-ID: 0a0b152083cfc44ec1bb599b57b7aab41327f998 Gitweb: https://git.kernel.org/tip/0a0b152083cfc44ec1bb599b57b7aab41327f998 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:51 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys: Override pkey when moving away from PROT_EXEC
I got a bug report that the following code (roughly) was causing a SIGSEGV:
mprotect(ptr, size, PROT_EXEC); mprotect(ptr, size, PROT_NONE); mprotect(ptr, size, PROT_READ); *ptr = 100;
The problem is hit when the mprotect(PROT_EXEC)
is implicitly assigned a protection key to the VMA, and made that key ACCESS_DENY|WRITE_DENY. The PROT_NONE mprotect() failed to remove the protection key, and the PROT_NONE-> PROT_READ left the PTE usable, but the pkey still in place and left the memory inaccessible.
To fix this, we ensure that we always "override" the pkee at mprotect() if the VMA does not have execute-only permissions, but the VMA has the execute-only pkey.
We had a check for PROT_READ/WRITE, but it did not work for PROT_NONE. This entirely removes the PROT_* checks, which ensures that PROT_NONE now works.
--- arch/x86/include/asm/pkeys.h | 12 +++++++++++- arch/x86/mm/pkeys.c | 21 +++++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-)
arch/x86/include/asm/pkeys.h +11 -1
@@ -2,6 +2,8 @@#ifndef _ASM_X86_PKEYS_H#define _ASM_X86_PKEYS_H +#define ARCH_DEFAULT_PKEY 0 +#define arch_max_pkey () (boot_cpu_has(X86_FEATURE_OSPKE) ? 16 : 1) extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
@@ -15,7 +17,7 @@ extern int __execute_only_pkey(struct mm_struct *mm);static inline int execute_only_pkey (struct mm_struct *mm) {if (!boot_cpu_has (X86_FEATURE_OSPKE )) -return 0 ; +return ARCH_DEFAULT_PKEY ;return __execute_only_pkey (mm); }
@@ -56,6 +58,14 @@ bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)return false;if (pkey >=arch_max_pkey ())return false; +/* +* The exec-only pkey is set in the allocation map, but +* is not available to any of the user interfaces like +* mprotect_pkey(). +*/ +if (pkey == mm->context.execute_only_pkey) +return false; +return mm_pkey_allocation_map (mm) & (1U << pkey); }
arch/x86/mm/pkeys.c +11 -10
@@ -94,26 +94,27 @@ int __arch_override_mprotect_pkey(struct vm_area_struct *vma, int prot, int pkey*/ if (pkey !=-1 )return pkey; -/* -* Look for a protection-key-drive execute-only mapping -* which is now being given permissions that are not -* execute-only. Move it back to the default pkey. -*/ -if (vma_is_pkey_exec_only (vma) && - (prot & (PROT_READ |PROT_WRITE ))) { -return 0 ; - } +/* * The mapping is execute-only. Go try to get the * execute-only protection key. If we fail to do that, * fall through as if we do not have execute-only -* support. +* support in this mm. */ if (prot ==PROT_EXEC ) { pkey =execute_only_pkey (vma->vm_mm);if (pkey >0 )return pkey; + }else if (vma_is_pkey_exec_only (vma)) { +/* +* Protections are *not* PROT_EXEC, but the mapping +* is using the exec-only pkey. This mapping was +* PROT_EXEC and will no longer be. Move back to +* the default pkey. +*/ +return ARCH_DEFAULT_PKEY ; } +/* * This is a vanilla, non-pkey mprotect (or we failed to * setup execute-only), inherit the pkey from the VMA we
Dave Hansen
maintainerPATCH v4 10/13 x86/pkeys/selftests: Fix pointer math
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
We dump out the entire area of the siginfo where the si_pkey_ptr is supposed to be. But, we do some math on the poitner, which is a u32. We intended to do byte math, not u32 math on the pointer.
Cast it over to a u8* so it works.
Also, move this block of code to below th si_code check. It doesn't hurt anything, but the si_pkey field is gibberish for other signal types.
---
b/tools/testing/selftests/x86/protection_keys.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-fix-pointer-math +7 -7
@@ -293,13 +293,6 @@ void signal_handler(int signum, siginfo_dump_mem (pkru_ptr -128 ,256 );pkey_assert (*pkru_ptr); - si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset); -dprintf1 ("si_pkey_ptr: %p\n" , si_pkey_ptr); -dump_mem (si_pkey_ptr -8 ,24 ); - siginfo_pkey = *si_pkey_ptr; -pkey_assert (siginfo_pkey <NR_PKEYS ); - last_si_pkey = siginfo_pkey; -if ((si->si_code ==SEGV_MAPERR ) || (si->si_code ==SEGV_ACCERR ) || (si->si_code ==SEGV_BNDERR )) {
@@ -307,6 +300,13 @@ void signal_handler(int signum, siginfo_exit (4 ); } + si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset); +dprintf1 ("si_pkey_ptr: %p\n" , si_pkey_ptr); +dump_mem ((u8 *)si_pkey_ptr -8 ,24 ); + siginfo_pkey = *si_pkey_ptr; +pkey_assert (siginfo_pkey <NR_PKEYS ); + last_si_pkey = siginfo_pkey; +dprintf1 ("signal pkru from xsave: %08x\n" , *pkru_ptr);/* need __rdpkru() version so we do not do shadow_pkru checking */ dprintf1 ("signal pkru from pkru: %08x\n" ,__rdpkru ());
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Fix pointer math
x86/pkeys/selftests: Fix pointer math
Commit-ID: 3d64f4ed15c3c53dba4c514bf59c334464dee373 Gitweb: https://git.kernel.org/tip/3d64f4ed15c3c53dba4c514bf59c334464dee373 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:52 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Fix pointer math
We dump out the entire area of the siginfo where the si_pkey_ptr is supposed to be. But, we do some math on the poitner, which is a u32. We intended to do byte math, not u32 math on the pointer.
Cast it over to a u8* so it works.
Also, move this block of code to below th si_code check. It doesn't hurt anything, but the si_pkey field is gibberish for other signal types.
--- tools/testing/selftests/x86/protection_keys.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
tools/testing/selftests/x86/protection_keys.c +7 -7
@@ -303,13 +303,6 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)dump_mem (pkru_ptr -128 ,256 );pkey_assert (*pkru_ptr); - si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset); -dprintf1 ("si_pkey_ptr: %p\n" , si_pkey_ptr); -dump_mem (si_pkey_ptr -8 ,24 ); - siginfo_pkey = *si_pkey_ptr; -pkey_assert (siginfo_pkey <NR_PKEYS ); - last_si_pkey = siginfo_pkey; -if ((si->si_code ==SEGV_MAPERR ) || (si->si_code ==SEGV_ACCERR ) || (si->si_code ==SEGV_BNDERR )) {
@@ -317,6 +310,13 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)exit (4 ); } + si_pkey_ptr = (u32 *)(((u8 *)si) + si_pkey_offset); +dprintf1 ("si_pkey_ptr: %p\n" , si_pkey_ptr); +dump_mem ((u8 *)si_pkey_ptr -8 ,24 ); + siginfo_pkey = *si_pkey_ptr; +pkey_assert (siginfo_pkey <NR_PKEYS ); + last_si_pkey = siginfo_pkey; +dprintf1 ("signal pkru from xsave: %08x\n" , *pkru_ptr);/* need __rdpkru() version so we do not do shadow_pkru checking */ dprintf1 ("signal pkru from pkru: %08x\n" ,__rdpkru ());
Dave Hansen
maintainerPATCH v4 11/13 x86/pkeys/selftests: Save off 'prot' for allocations
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
This makes it possible to to tell what 'prot' a given allocation is supposed to have. That way, if we want to change just the pkey, we know what 'prot' to pass to mprotect_pkey().
Also, keep a record of the most recent allocation so the tests can easily find it.
---
b/tools/testing/selftests/x86/protection_keys.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-update-selftests-store-malloc-record +9 -5
@@ -662,10 +662,12 @@ int mprotect_pkey(void *ptr, size_t sizestruct pkey_malloc_record {void *ptr;long size; +int prot; };struct pkey_malloc_record *pkey_malloc_records; +struct pkey_malloc_record *pkey_last_malloc_record;long nr_pkey_malloc_records; -void record_pkey_malloc (void *ptr,long size) +void record_pkey_malloc (void *ptr,long size,int prot) {long i;struct pkey_malloc_record *rec =NULL ;
@@ -697,6 +699,8 @@ void record_pkey_malloc(void *ptr, long (int)(rec - pkey_malloc_records), rec, ptr, size); rec->ptr = ptr; rec->size = size; + rec->prot = prot; + pkey_last_malloc_record = rec; nr_pkey_malloc_records++; }
@@ -741,7 +745,7 @@ void *malloc_pkey_with_mprotect(long sizpkey_assert (ptr != (void *)-1 ); ret =mprotect_pkey ((void *)ptr,PAGE_SIZE , prot, pkey);pkey_assert (!ret); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);rdpkru ();dprintf1 ("%s() for pkey %d @ %p\n" , __func__, pkey, ptr);
@@ -762,7 +766,7 @@ void *malloc_pkey_anon_huge(long size, i size =ALIGN_UP (size,HPAGE_SIZE *2 ); ptr =mmap (NULL , size,PROT_NONE ,MAP_ANONYMOUS |MAP_PRIVATE ,-1 ,0 );pkey_assert (ptr != (void *)-1 ); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);mprotect_pkey (ptr, size, prot, pkey);dprintf1 ("unaligned ptr: %p\n" , ptr);
@@ -835,7 +839,7 @@ void *malloc_pkey_hugetlb(long size, intpkey_assert (ptr != (void *)-1 );mprotect_pkey (ptr, size, prot, pkey); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);dprintf1 ("mmap()'d hugetlbfs for pkey %d @ %p\n" , pkey, ptr);return ptr;
@@ -857,7 +861,7 @@ void *malloc_pkey_mmap_dax(long size, inmprotect_pkey (ptr, size, prot, pkey); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);dprintf1 ("mmap()'d for pkey %d @ %p\n" , pkey, ptr);close (fd);
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Save off 'prot' for allocations
x86/pkeys/selftests: Save off 'prot' for allocations
Commit-ID: acb25d761d6f2f64e785ccefc71e54f244f1eda4 Gitweb: https://git.kernel.org/tip/acb25d761d6f2f64e785ccefc71e54f244f1eda4 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:54 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Save off 'prot' for allocations
This makes it possible to to tell what 'prot' a given allocation is supposed to have. That way, if we want to change just the pkey, we know what 'prot' to pass to mprotect_pkey().
Also, keep a record of the most recent allocation so the tests can easily find it.
--- tools/testing/selftests/x86/protection_keys.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c +9 -5
@@ -677,10 +677,12 @@ int mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,struct pkey_malloc_record {void *ptr;long size; +int prot; };struct pkey_malloc_record *pkey_malloc_records; +struct pkey_malloc_record *pkey_last_malloc_record;long nr_pkey_malloc_records; -void record_pkey_malloc (void *ptr,long size) +void record_pkey_malloc (void *ptr,long size,int prot) {long i;struct pkey_malloc_record *rec =NULL ;
@@ -712,6 +714,8 @@ void record_pkey_malloc(void *ptr, long size) (int)(rec - pkey_malloc_records), rec, ptr, size); rec->ptr = ptr; rec->size = size; + rec->prot = prot; + pkey_last_malloc_record = rec; nr_pkey_malloc_records++; }
@@ -756,7 +760,7 @@ void *malloc_pkey_with_mprotect(long size, int prot, u16 pkey)pkey_assert (ptr != (void *)-1 ); ret =mprotect_pkey ((void *)ptr,PAGE_SIZE , prot, pkey);pkey_assert (!ret); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);rdpkru ();dprintf1 ("%s() for pkey %d @ %p\n" , __func__, pkey, ptr);
@@ -777,7 +781,7 @@ void *malloc_pkey_anon_huge(long size, int prot, u16 pkey) size =ALIGN_UP (size,HPAGE_SIZE *2 ); ptr =mmap (NULL , size,PROT_NONE ,MAP_ANONYMOUS |MAP_PRIVATE ,-1 ,0 );pkey_assert (ptr != (void *)-1 ); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);mprotect_pkey (ptr, size, prot, pkey);dprintf1 ("unaligned ptr: %p\n" , ptr);
@@ -850,7 +854,7 @@ void *malloc_pkey_hugetlb(long size, int prot, u16 pkey)pkey_assert (ptr != (void *)-1 );mprotect_pkey (ptr, size, prot, pkey); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);dprintf1 ("mmap()'d hugetlbfs for pkey %d @ %p\n" , pkey, ptr);return ptr;
@@ -872,7 +876,7 @@ void *malloc_pkey_mmap_dax(long size, int prot, u16 pkey)mprotect_pkey (ptr, size, prot, pkey); -record_pkey_malloc (ptr, size); +record_pkey_malloc (ptr, size, prot);dprintf1 ("mmap()'d for pkey %d @ %p\n" , pkey, ptr);close (fd);
Dave Hansen
maintainerPATCH v4 12/13 x86/pkeys/selftests: Add a test for pkey 0
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
Protection key 0 is the default key for all memory and will not normally come back from pkey_alloc(). But, you might still want pass it to mprotect_pkey().
This check ensures that you can use pkey 0.
---
b/tools/testing/selftests/x86/protection_keys.c | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~pkeys-update-selftests-with-pkey-0-test +30 -0
@@ -1169,6 +1169,35 @@ void test_pkey_alloc_exhaust(int *ptr, u } } +/* +* pkey 0 is special. It is allocated by default, so you do not +* have to call pkey_alloc() to use it first. Make sure that it +* is usable. +*/ +void test_mprotect_with_pkey_0 (int *ptr,u16 pkey) +{ +long size; +int prot; + +assert (pkey_last_malloc_record); + size = pkey_last_malloc_record->size; +/* +* This is a bit of a hack. But mprotect() requires +* huge-page-aligned sizes when operating on hugetlbfs. +* So, make sure that we use something that's a multiple +* of a huge page when we can. +*/ +if (size >=HPAGE_SIZE ) + size =HPAGE_SIZE ; + prot = pkey_last_malloc_record->prot; + +/* Use pkey 0 */ +mprotect_pkey (ptr, size, prot,0 ); + +/* Make sure that we can set it back to the original pkey. */ +mprotect_pkey (ptr, size, prot, pkey); +} +void test_ptrace_of_child (int *ptr,u16 pkey) { __attribute__((__unused__))int peek_result;
@@ -1363,6 +1392,7 @@ void (*pkey_tests[])(int *ptr, u16 pkey) test_kernel_gup_write_to_write_disabled_region, test_executing_on_unreadable_memory, test_implicit_mprotect_exec_only_memory, + test_mprotect_with_pkey_0, test_ptrace_of_child, test_pkey_syscalls_on_non_allocated_pkey, test_pkey_syscalls_bad_args,
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys/selftests: Add a test for pkey 0
x86/pkeys/selftests: Add a test for pkey 0
Commit-ID: 3488a600d90bcaf061b104dbcfbdc8d99b398312 Gitweb: https://git.kernel.org/tip/3488a600d90bcaf061b104dbcfbdc8d99b398312 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:56 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Add a test for pkey 0
Protection key 0 is the default key for all memory and will not normally come back from pkey_alloc(). But, you might still want pass it to mprotect_pkey().
This check ensures that you can use pkey 0.
--- tools/testing/selftests/x86/protection_keys.c | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
tools/testing/selftests/x86/protection_keys.c +30 -0
@@ -1184,6 +1184,35 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey) } } +/* +* pkey 0 is special. It is allocated by default, so you do not +* have to call pkey_alloc() to use it first. Make sure that it +* is usable. +*/ +void test_mprotect_with_pkey_0 (int *ptr,u16 pkey) +{ +long size; +int prot; + +assert (pkey_last_malloc_record); + size = pkey_last_malloc_record->size; +/* +* This is a bit of a hack. But mprotect() requires +* huge-page-aligned sizes when operating on hugetlbfs. +* So, make sure that we use something that's a multiple +* of a huge page when we can. +*/ +if (size >=HPAGE_SIZE ) + size =HPAGE_SIZE ; + prot = pkey_last_malloc_record->prot; + +/* Use pkey 0 */ +mprotect_pkey (ptr, size, prot,0 ); + +/* Make sure that we can set it back to the original pkey. */ +mprotect_pkey (ptr, size, prot, pkey); +} +void test_ptrace_of_child (int *ptr,u16 pkey) { __attribute__((__unused__))int peek_result;
@@ -1378,6 +1407,7 @@ void (*pkey_tests[])(int *ptr, u16 pkey) = { test_kernel_gup_write_to_write_disabled_region, test_executing_on_unreadable_memory, test_implicit_mprotect_exec_only_memory, + test_mprotect_with_pkey_0, test_ptrace_of_child, test_pkey_syscalls_on_non_allocated_pkey, test_pkey_syscalls_bad_args,
Dave Hansen
maintainerPATCH v4 13/13 x86/pkeys: Do not special case protection key 0
In reply to Dave HansenFrom: Dave Hansen <[email protected]>
mm_pkey_is_allocated() treats pkey 0 as unallocated. That is inconsistent with the manpages, and also inconsistent with mm->context.pkey_allocation_map. Stop special casing it and only disallow values that are actually bad (< 0).
The end-user visible effect of this is that you can now use mprotect_pkey() to set pkey=0.
This is a bit nicer than what Ram proposed[1] because it is simpler and removes special-casing for pkey 0. On the other hand, it does allow applications to pkey_free() pkey-0, but that's just a silly thing to do, so we are not going to protect against it.
The scenario that could happen is similar to what happens if you free any other pkey that is in use: it might get reallocated later and used to protect some other data. The most likely scenario is that pkey-0 comes back from pkey_alloc(), an access-disable or write-disable bit is set in PKRU for it, and the next stack access will SIGSEGV. It's not horribly different from if you mprotect()'d your stack or heap to be unreadable or unwritable, which is generally very foolish, but also not explicitly prevented by the kernel.
1. http://lkml.kernel.org/r/[email protected]
---
b/arch/x86/include/asm/mmu_context.h | 2 +- b/arch/x86/include/asm/pkeys.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
arch/x86/include/asm/mmu_context.h renamed from arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated +1 -1
@@ -193,7 +193,7 @@ static inline int init_new_context(struc#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS if (cpu_feature_enabled (X86_FEATURE_OSPKE )) { -/* pkey 0 is the default and always allocated */ +/* pkey 0 is the default and allocated implicitly */ mm->context.pkey_allocation_map =0x1 ;/* -1 means unallocated or invalid */ mm->context.execute_only_pkey =-1 ;
arch/x86/include/asm/pkeys.h renamed from arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated +3 -3
@@ -51,10 +51,10 @@ bool mm_pkey_is_allocated(struct mm_stru {/* * "Allocated" pkeys are those that have been returned -* from pkey_alloc(). pkey 0 is special, and never -* returned from pkey_alloc(). +* from pkey_alloc() or pkey 0 which is allocated +* implicitly when the mm is created. */ -if (pkey <=0 ) +if (pkey <0 )return false;if (pkey >=arch_max_pkey ())return false;
_
tip-bot for Dave Hansen
merge bot[tip:x86/urgent] x86/pkeys: Do not special case protection key 0
x86/pkeys: Do not special case protection key 0
Commit-ID: 2fa9d1cfaf0e02f8abef0757002bff12dfcfa4e6 Gitweb: https://git.kernel.org/tip/2fa9d1cfaf0e02f8abef0757002bff12dfcfa4e6 Author: Dave Hansen <[email protected]> AuthorDate: Wed, 9 May 2018 10:13:58 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys: Do not special case protection key 0
mm_pkey_is_allocated() treats pkey 0 as unallocated. That is inconsistent with the manpages, and also inconsistent with mm->context.pkey_allocation_map. Stop special casing it and only disallow values that are actually bad (< 0).
The end-user visible effect of this is that you can now use mprotect_pkey() to set pkey=0.
This is a bit nicer than what Ram proposed[1] because it is simpler and removes special-casing for pkey 0. On the other hand, it does allow applications to pkey_free() pkey-0, but that's just a silly thing to do, so we are not going to protect against it.
The scenario that could happen is similar to what happens if you free any other pkey that is in use: it might get reallocated later and used to protect some other data. The most likely scenario is that pkey-0 comes back from pkey_alloc(), an access-disable or write-disable bit is set in PKRU for it, and the next stack access will SIGSEGV. It's not horribly different from if you mprotect()'d your stack or heap to be unreadable or unwritable, which is generally very foolish, but also not explicitly prevented by the kernel.
1. http://lkml.kernel.org/r/[email protected]
--- arch/x86/include/asm/mmu_context.h | 2 +- arch/x86/include/asm/pkeys.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
arch/x86/include/asm/mmu_context.h +1 -1
@@ -193,7 +193,7 @@ static inline int init_new_context(struct task_struct *tsk,#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS if (cpu_feature_enabled (X86_FEATURE_OSPKE )) { -/* pkey 0 is the default and always allocated */ +/* pkey 0 is the default and allocated implicitly */ mm->context.pkey_allocation_map =0x1 ;/* -1 means unallocated or invalid */ mm->context.execute_only_pkey =-1 ;
arch/x86/include/asm/pkeys.h +3 -3
@@ -51,10 +51,10 @@ bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey) {/* * "Allocated" pkeys are those that have been returned -* from pkey_alloc(). pkey 0 is special, and never -* returned from pkey_alloc(). +* from pkey_alloc() or pkey 0 which is allocated +* implicitly when the mm is created. */ -if (pkey <=0 ) +if (pkey <0 )return false;if (pkey >=arch_max_pkey ())return false;
Quoted 51 lines · Dave Hansen
Hi x86 maintainers,
This set has been seen quite a few changes and additions since the last post. Details below.
Changes from v3:
* Reordered patches following Ingo's recommendations: Introduce failing selftests first, then the kernel code to fix the test failure.
* Increase verbosity and accuracy ofdo_not_expect_pk_fault()messages.
* Removedabort()use from tests. Crashing is not nice.
* Remove some dead debugging code, fixingdprint_in_signal.
* Fix deadlocks from usingprintf()and friends in signal handlers.Changes from v2:
* Clarified commit message in patch 1/9 taking some feedback from Shuah.Changes from v1:
* Added Fixes: and cc'd stable. No code changes.--
This fixes two bugs, and adds selftests to make sure they stay fixed:
1. pkey 0 was not usable via
mprotect_pkey()because it had never been explicitly allocated.
2.mprotect(PROT_EXEC)memory could sometimes be left with the implicit exec-only protection key assigned.I already posted #1 previously. I'm including them both here because I don't think it's been picked up in case folks want to pull these all in a single bundle.
Dave Hansen (13):
x86/pkeys/selftests: give better unexpected fault error messages x86/pkeys/selftests: Stop usingassert()x86/pkeys/selftests: remove dead debugging code, fixdprint_in_signalx86/pkeys/selftests: avoid printf-in-signal deadlocks x86/pkeys/selftests: Allow faults on unknown keys x86/pkeys/selftests: Factor out "instruction page" x86/pkeys/selftests: AddPROT_EXECtest x86/pkeys/selftests: Fix pkey exhaustion test off-by-one x86/pkeys: Override pkey when moving away fromPROT_EXECx86/pkeys/selftests: Fix pointer math x86/pkeys/selftests: Save off 'prot' for allocations x86/pkeys/selftests: Add a test for pkey 0
x86/pkeys: Do not special case protection key 0
Quoted 6 lines
arch/x86/include/asm/mmu_context.h | 2 +- arch/x86/include/asm/pkeys.h | 18 +- arch/x86/mm/pkeys.c | 21 +- tools/testing/selftests/x86/pkey-helpers.h | 20 +- tools/testing/selftests/x86/protection_keys.c | 187 +++++++++++++----- 5 files changed, 173 insertions(+), 75 deletions(-)
So this series is looking good to me in principle, but trying to build it I got warnings and errors - see the build log below.
Note that this is on a box with "Ubuntu 18.04 LTS (Bionic Beaver)".
Thanks,
Ingo
================>
gcc -m32 -o /home/mingo/tip/tools/testing/selftests/x86/protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall -no-pie -DCAN_BUILD_32 -DCAN_BUILD_64 protection_keys.c -lrt -ldl -lm protection_keys.c:232:0: warning: "SEGV_BNDERR" redefined
#define SEGV_BNDERR 3 /* failed address bound checks */
In file included from /usr/include/signal.h:58:0,
from protection_keys.c:33:/usr/include/bits/siginfo-consts.h:117:0: note: this is the location of the previous definition
# define SEGV_BNDERR SEGV_BNDERR
protection_keys.c:233:0: warning: "SEGV_PKUERR" redefined
#define SEGV_PKUERR 4
In file included from /usr/include/signal.h:58:0,
from protection_keys.c:33:/usr/include/bits/siginfo-consts.h:119:0: note: this is the location of the previous definition
# define SEGV_PKUERR SEGV_PKUERR
protection_keys.c:387:5: error: conflicting types for a??pkey_geta??
u32 pkey_get(int pkey, unsigned long flags)
^~~~~~~~
In file included from /usr/include/bits/mman-linux.h:115:0,
from /usr/include/bits/mman.h:45,
from /usr/include/sys/mman.h:41,
from protection_keys.c:37:
/usr/include/bits/mman-shared.h:64:5: note: previous declaration of a??pkey_geta?? was here
int pkey_get (int __key) __THROW;
^~~~~~~~
protection_keys.c:409:5: error: conflicting types for a??pkey_seta??
int pkey_set(int pkey, unsigned long rights, unsigned long flags)
^~~~~~~~
In file included from /usr/include/bits/mman-linux.h:115:0,
from /usr/include/bits/mman.h:45,
from /usr/include/sys/mman.h:41,
from protection_keys.c:37:
/usr/include/bits/mman-shared.h:60:5: note: previous declaration of a??pkey_seta?? was here
int pkey_set (int __key, unsigned int __access_rights) __THROW;
^~~~~~~~Makefile:67: recipe for target '/home/mingo/tip/tools/testing/selftests/x86/protection_keys_32' failed
make: *** [/home/mingo/tip/tools/testing/selftests/x86/protection_keys_32] Error 1
Quoted 4 lines · Ingo Molnar
So this series is looking good to me in principle, but trying to build it I got warnings and errors - see the build log below.
Note that this is on a box with "Ubuntu 18.04 LTS (Bionic Beaver)".
So it turns out that the build errors already exist without the series applied, so it must be some interaction between the pkeys self-tests and latest Ubuntu.
Thanks,
Ingo
PATCH 1/1 x86/pkeys/selftests: Adjust the self-test to fresh distros that export the pkeys ABI
In reply to Ingo MolnarUbuntu 18.04 started exporting pkeys details in header files, resulting in build failures and warnings in the pkeys self-tests:
protection_keys.c:232:0: warning: "SEGV_BNDERR" redefined protection_keys.c:387:5: error: conflicting types for a??pkey_geta?? protection_keys.c:409:5: error: conflicting types for a??pkey_seta??
...
Fix these namespace conflicts and double definitions, plus also clean up the ABI definitions to make it all a bit more readable ...
--- tools/testing/selftests/x86/protection_keys.c | 67 ++++++++++++++++----------- 1 file changed, 41 insertions(+), 26 deletions(-)
tools/testing/selftests/x86/protection_keys.c +41 -26
@@ -191,26 +191,30 @@ void lots_o_noops_around_write(int *write_to_me)#ifdef __i386__#ifndef SYS_mprotect_key -# define SYS_mprotect_key 380 +# define SYS_mprotect_key 380#endif +#ifndef SYS_pkey_alloc -# define SYS_pkey_alloc 381 -# define SYS_pkey_free 382 +# define SYS_pkey_alloc 381 +# define SYS_pkey_free 382#endif -#define REG_IP_IDX REG_EIP -#define si_pkey_offset 0x14 + +#define REG_IP_IDX REG_EIP +#define si_pkey_offset 0x14#else #ifndef SYS_mprotect_key -# define SYS_mprotect_key 329 +# define SYS_mprotect_key 329#endif +#ifndef SYS_pkey_alloc -# define SYS_pkey_alloc 330 -# define SYS_pkey_free 331 +# define SYS_pkey_alloc 330 +# define SYS_pkey_free 331#endif -#define REG_IP_IDX REG_RIP -#define si_pkey_offset 0x20 + +#define REG_IP_IDX REG_RIP +#define si_pkey_offset 0x20#endif
@@ -225,8 +229,14 @@ void dump_mem(void *dumpme, int len_bytes) } } -#define SEGV_BNDERR 3/* failed address bound checks */ -#define SEGV_PKUERR 4 +/* Failed address bound checks: */ +#ifndef SEGV_BNDERR +# define SEGV_BNDERR 3 +#endif + +#ifndef SEGV_PKUERR +# define SEGV_PKUERR 4 +#endif static char *si_code_str (int si_code) {
@@ -393,10 +403,15 @@ pid_t fork_lazy_child(void)return forkret; } -#define PKEY_DISABLE_ACCESS 0x1 -#define PKEY_DISABLE_WRITE 0x2 +#ifndef PKEY_DISABLE_ACCESS +# define PKEY_DISABLE_ACCESS 0x1 +#endif + +#ifndef PKEY_DISABLE_WRITE +# define PKEY_DISABLE_WRITE 0x2 +#endif -u32 pkey_get (int pkey,unsigned long flags) +static u32 hw_pkey_get (int pkey,unsigned long flags) {u32 mask = (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );u32 pkru =__rdpkru ();
@@ -418,7 +433,7 @@ u32 pkey_get(int pkey, unsigned long flags)return masked_pkru; } -int pkey_set (int pkey,unsigned long rights,unsigned long flags) +static int hw_pkey_set (int pkey,unsigned long rights,unsigned long flags) {u32 mask = (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );u32 old_pkru =__rdpkru ();
@@ -452,15 +467,15 @@ void pkey_disable_set(int pkey, int flags) pkey, flags);pkey_assert (flags & (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE )); - pkey_rights =pkey_get (pkey, syscall_flags); + pkey_rights =hw_pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);pkey_assert (pkey_rights >=0 ); pkey_rights |= flags; - ret =pkey_set (pkey, pkey_rights, syscall_flags); + ret =hw_pkey_set (pkey, pkey_rights, syscall_flags);assert (!ret);/*pkru and flags have the same format */ shadow_pkru |= flags << (pkey *2 );
@@ -468,8 +483,8 @@ void pkey_disable_set(int pkey, int flags)pkey_assert (ret >=0 ); - pkey_rights =pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, + pkey_rights =hw_pkey_get (pkey, syscall_flags); +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);dprintf1 ("%s(%d) pkru: 0x%x\n" , __func__, pkey,rdpkru ());
@@ -483,24 +498,24 @@ void pkey_disable_clear(int pkey, int flags) {unsigned long syscall_flags =0 ;int ret; -int pkey_rights =pkey_get (pkey, syscall_flags); +int pkey_rights =hw_pkey_get (pkey, syscall_flags);u32 orig_pkru =rdpkru ();pkey_assert (flags & (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE )); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);pkey_assert (pkey_rights >=0 ); pkey_rights |= flags; - ret =pkey_set (pkey, pkey_rights,0 ); + ret =hw_pkey_set (pkey, pkey_rights,0 );/* pkru and flags have the same format */ shadow_pkru &= ~(flags << (pkey *2 ));pkey_assert (ret >=0 ); - pkey_rights =pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, + pkey_rights =hw_pkey_get (pkey, syscall_flags); +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);dprintf1 ("%s(%d) pkru: 0x%x\n" , __func__, pkey,rdpkru ());
tip-bot for Ingo Molnar
merge bot[tip:x86/urgent] x86/pkeys/selftests: Adjust the self-test to fresh distros that export the pkeys ABI
x86/pkeys/selftests: Adjust the self-test to fresh distros that export the pkeys ABI
Commit-ID: 0fb96620dce351608aa82eed5942e2f58b07beda Gitweb: https://git.kernel.org/tip/0fb96620dce351608aa82eed5942e2f58b07beda Author: Ingo Molnar <[email protected]> AuthorDate: Mon, 14 May 2018 10:56:23 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/pkeys/selftests: Adjust the self-test to fresh distros that export the pkeys ABI
Ubuntu 18.04 started exporting pkeys details in header files, resulting in build failures and warnings in the pkeys self-tests:
protection_keys.c:232:0: warning: "SEGV_BNDERR" redefined protection_keys.c:387:5: error: conflicting types for ‘pkey_get’ protection_keys.c:409:5: error: conflicting types for ‘pkey_set’
...
Fix these namespace conflicts and double definitions, plus also clean up the ABI definitions to make it all a bit more readable ...
--- tools/testing/selftests/x86/protection_keys.c | 67 ++++++++++++++++----------- 1 file changed, 41 insertions(+), 26 deletions(-)
tools/testing/selftests/x86/protection_keys.c +41 -26
@@ -191,26 +191,30 @@ void lots_o_noops_around_write(int *write_to_me)#ifdef __i386__#ifndef SYS_mprotect_key -# define SYS_mprotect_key 380 +# define SYS_mprotect_key 380#endif +#ifndef SYS_pkey_alloc -# define SYS_pkey_alloc 381 -# define SYS_pkey_free 382 +# define SYS_pkey_alloc 381 +# define SYS_pkey_free 382#endif -#define REG_IP_IDX REG_EIP -#define si_pkey_offset 0x14 + +#define REG_IP_IDX REG_EIP +#define si_pkey_offset 0x14#else #ifndef SYS_mprotect_key -# define SYS_mprotect_key 329 +# define SYS_mprotect_key 329#endif +#ifndef SYS_pkey_alloc -# define SYS_pkey_alloc 330 -# define SYS_pkey_free 331 +# define SYS_pkey_alloc 330 +# define SYS_pkey_free 331#endif -#define REG_IP_IDX REG_RIP -#define si_pkey_offset 0x20 + +#define REG_IP_IDX REG_RIP +#define si_pkey_offset 0x20#endif
@@ -225,8 +229,14 @@ void dump_mem(void *dumpme, int len_bytes) } } -#define SEGV_BNDERR 3/* failed address bound checks */ -#define SEGV_PKUERR 4 +/* Failed address bound checks: */ +#ifndef SEGV_BNDERR +# define SEGV_BNDERR 3 +#endif + +#ifndef SEGV_PKUERR +# define SEGV_PKUERR 4 +#endif static char *si_code_str (int si_code) {
@@ -393,10 +403,15 @@ pid_t fork_lazy_child(void)return forkret; } -#define PKEY_DISABLE_ACCESS 0x1 -#define PKEY_DISABLE_WRITE 0x2 +#ifndef PKEY_DISABLE_ACCESS +# define PKEY_DISABLE_ACCESS 0x1 +#endif + +#ifndef PKEY_DISABLE_WRITE +# define PKEY_DISABLE_WRITE 0x2 +#endif -u32 pkey_get (int pkey,unsigned long flags) +static u32 hw_pkey_get (int pkey,unsigned long flags) {u32 mask = (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );u32 pkru =__rdpkru ();
@@ -418,7 +433,7 @@ u32 pkey_get(int pkey, unsigned long flags)return masked_pkru; } -int pkey_set (int pkey,unsigned long rights,unsigned long flags) +static int hw_pkey_set (int pkey,unsigned long rights,unsigned long flags) {u32 mask = (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );u32 old_pkru =__rdpkru ();
@@ -452,15 +467,15 @@ void pkey_disable_set(int pkey, int flags) pkey, flags);pkey_assert (flags & (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE )); - pkey_rights =pkey_get (pkey, syscall_flags); + pkey_rights =hw_pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);pkey_assert (pkey_rights >=0 ); pkey_rights |= flags; - ret =pkey_set (pkey, pkey_rights, syscall_flags); + ret =hw_pkey_set (pkey, pkey_rights, syscall_flags);assert (!ret);/*pkru and flags have the same format */ shadow_pkru |= flags << (pkey *2 );
@@ -468,8 +483,8 @@ void pkey_disable_set(int pkey, int flags)pkey_assert (ret >=0 ); - pkey_rights =pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, + pkey_rights =hw_pkey_get (pkey, syscall_flags); +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);dprintf1 ("%s(%d) pkru: 0x%x\n" , __func__, pkey,rdpkru ());
@@ -483,24 +498,24 @@ void pkey_disable_clear(int pkey, int flags) {unsigned long syscall_flags =0 ;int ret; -int pkey_rights =pkey_get (pkey, syscall_flags); +int pkey_rights =hw_pkey_get (pkey, syscall_flags);u32 orig_pkru =rdpkru ();pkey_assert (flags & (PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE )); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);pkey_assert (pkey_rights >=0 ); pkey_rights |= flags; - ret =pkey_set (pkey, pkey_rights,0 ); + ret =hw_pkey_set (pkey, pkey_rights,0 );/* pkru and flags have the same format */ shadow_pkru &= ~(flags << (pkey *2 ));pkey_assert (ret >=0 ); - pkey_rights =pkey_get (pkey, syscall_flags); -dprintf1 ("%s(%d) pkey_get(%d): %x\n" , __func__, + pkey_rights =hw_pkey_get (pkey, syscall_flags); +dprintf1 ("%s(%d) hw_pkey_get(%d): %x\n" , __func__, pkey, pkey, pkey_rights);dprintf1 ("%s(%d) pkru: 0x%x\n" , __func__, pkey,rdpkru ());
PATCH 1/1 x86/mpx/selftests: Adjust the self-test to fresh distros that export the MPX ABI
In reply to Ingo MolnarFix this warning:
mpx-mini-test.c:422:0: warning: "SEGV_BNDERR" redefined
--- tools/testing/selftests/x86/mpx-mini-test.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
tools/testing/selftests/x86/mpx-mini-test.c +5 -2
@@ -368,6 +368,11 @@ static int expected_bnd_index = -1;uint64_t shadow_plb[NR_MPX_BOUNDS_REGISTERS ][2 ];/* shadow MPX bound registers */ unsigned long shadow_map[NR_MPX_BOUNDS_REGISTERS ]; +/* Failed address bound checks: */ +#ifndef SEGV_BNDERR +# define SEGV_BNDERR 3 +#endif +/* * The kernel is supposed to provide some information about the bounds * exception in the siginfo. It should match what we have in the bounds
@@ -419,8 +424,6 @@ void handler(int signum, siginfo_t *si, void *vucontext) br_count++;dprintf1 ("#BR 0x%jx (total seen: %d)\n" , status, br_count); -#define SEGV_BNDERR 3/* failed address bound checks */ -dprintf2 ("Saw a #BR! status 0x%jx at %016lx br_reason: %jx\n" , status, ip, br_reason);dprintf2 ("si_signo: %d\n" , si->si_signo);
tip-bot for Ingo Molnar
merge bot[tip:x86/urgent] x86/mpx/selftests: Adjust the self-test to fresh distros that export the MPX ABI
x86/mpx/selftests: Adjust the self-test to fresh distros that export the MPX ABI
Commit-ID: 73bb4d6cd192b8629c5125aaada9892d9fc986b6 Gitweb: https://git.kernel.org/tip/73bb4d6cd192b8629c5125aaada9892d9fc986b6 Author: Ingo Molnar <[email protected]> AuthorDate: Mon, 14 May 2018 10:59:08 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Mon, 14 May 2018 11:14:45 +0200
x86/mpx/selftests: Adjust the self-test to fresh distros that export the MPX ABI
Fix this warning:
mpx-mini-test.c:422:0: warning: "SEGV_BNDERR" redefined
--- tools/testing/selftests/x86/mpx-mini-test.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
tools/testing/selftests/x86/mpx-mini-test.c +5 -2
@@ -368,6 +368,11 @@ static int expected_bnd_index = -1;uint64_t shadow_plb[NR_MPX_BOUNDS_REGISTERS ][2 ];/* shadow MPX bound registers */ unsigned long shadow_map[NR_MPX_BOUNDS_REGISTERS ]; +/* Failed address bound checks: */ +#ifndef SEGV_BNDERR +# define SEGV_BNDERR 3 +#endif +/* * The kernel is supposed to provide some information about the bounds * exception in the siginfo. It should match what we have in the bounds
@@ -419,8 +424,6 @@ void handler(int signum, siginfo_t *si, void *vucontext) br_count++;dprintf1 ("#BR 0x%jx (total seen: %d)\n" , status, br_count); -#define SEGV_BNDERR 3/* failed address bound checks */ -dprintf2 ("Saw a #BR! status 0x%jx at %016lx br_reason: %jx\n" , status, ip, br_reason);dprintf2 ("si_signo: %d\n" , si->si_signo);