Skip to contentSkip to conversation

x86, pkeys: two protection keys bug fixes

PATCH In review Dave Hansen 3 participants 33 messages May 9, 2018 – May 14, 2018

Subsystems (5)
lore
Share & tools
lore

Dave Hansen

author maintainer
#0 lore raw

v4 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(-)
Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>p
Cc Shuah Khan <[email protected]>
Cc Shakeel Butt <[email protected]>
author maintainer
#1 lore raw

PATCH 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-give-better-pkey-fault-errors tools/testing/selftests/x86/protection_keys.c
@@ -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, u16
 	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);
@@ -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);

_

#2 lore raw

[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
index bbe80a5c31c7..a0f0a732784b 100644
@@ -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);
author maintainer
#3 lore raw

PATCH v4 2/13 x86/pkeys/selftests: Stop using assert()

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-do-not-assert tools/testing/selftests/x86/protection_keys.c
@@ -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);
 }

_

#4 lore raw

[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
index a0f0a732784b..8537a7cfe1cc 100644
@@ -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);
 }
author maintainer
#5 lore raw

PATCH v4 3/13 x86/pkeys/selftests: Remove dead debugging code, fix dprint_in_signal

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-remove-dead-code-after-return tools/testing/selftests/x86/protection_keys.c
@@ -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;
 }
 

_

#6 lore raw

[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
index 8537a7cfe1cc..5f5aedb80e7b 100644
@@ -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;
 }
 
author maintainer
#7 lore raw

PATCH v4 4/13 x86/pkeys/selftests: Avoid printf-in-signal deadlocks

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/pkey-helpers.h~pkeys-selftests-avoid-print-in-signal-deadlocks tools/testing/selftests/x86/pkey-helpers.h
@@ -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)

_

#8 lore raw

[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
index b3cb7670e026..254e5436bdd9 100644
@@ -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)
author maintainer
#9 lore raw

PATCH v4 5/13 x86/pkeys/selftests: Allow faults on unknown keys

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-unknown-exec-only-key tools/testing/selftests/x86/protection_keys.c
@@ -906,13 +906,21 @@ void *malloc_pkey(long size, int prot, u
 }
 
 int last_pkru_faults;
+#define UNKNOWN_PKEY -2
 void 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.

_

#10 lore raw

[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
index 5f5aedb80e7b..7d95acd2aec3 100644
@@ -921,13 +921,21 @@ void *malloc_pkey(long size, int prot, u16 pkey)
 }
 
 int last_pkru_faults;
+#define UNKNOWN_PKEY -2
 void 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.
author maintainer
#11 lore raw

PATCH v4 6/13 x86/pkeys/selftests: Factor out "instruction page"

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-get_pointer_to_instructions tools/testing/selftests/x86/protection_keys.c
@@ -1238,12 +1238,9 @@ void test_ptrace_of_child(int *ptr, u16
 	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);
@@ -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);

_

#12 lore raw

[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
index 7d95acd2aec3..083ebd51b44e 100644
@@ -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);
author maintainer
#13 lore raw

PATCH v4 7/13 x86/pkeys/selftests: Add PROT_EXEC test

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-prot_exec tools/testing/selftests/x86/protection_keys.c
@@ -1288,6 +1288,49 @@ void test_executing_on_unreadable_memory
 	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;
@@ -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,

_

#14 lore raw

[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
index 083ebd51b44e..8cb4fe58f381 100644
@@ -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,
author maintainer
#15 lore raw

PATCH v4 8/13 x86/pkeys/selftests: Fix pkey exhaustion test off-by-one

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-exhaust-off-by-one tools/testing/selftests/x86/protection_keys.c
@@ -1148,12 +1148,15 @@ void test_pkey_alloc_exhaust(int *ptr, u
 	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]);

_

#16 lore raw

[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
index 8cb4fe58f381..55a4e349a45e 100644
@@ -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]);
author maintainer
#17 lore raw

PATCH v4 9/13 x86/pkeys: Override pkey when moving away from PROT_EXEC

In reply to Dave Hansen

From: 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.

Fixes 62b5f7d013f ("mm/core, x86/mm/pkeys: Add execute-only protection keys support")
Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN arch/x86/include/asm/pkeys.h~pkeys-abandon-exec-only-pkey-more-aggressively arch/x86/include/asm/pkeys.h
@@ -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
 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_stru
 		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 renamed from arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively +11 -10
diff -puN arch/x86/mm/pkeys.c~pkeys-abandon-exec-only-pkey-more-aggressively arch/x86/mm/pkeys.c
@@ -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

_

#18 lore raw

[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.

Cc Andrew Morton <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ram Pai <[email protected]>
Cc Shuah Khan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes 62b5f7d013f ("mm/core, x86/mm/pkeys: Add execute-only protection keys support")
---
 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
index a0ba1ffda0df..39cd292805a9 100644
@@ -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
index d7bc0eea20a5..6e98e0a7c923 100644
@@ -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
author maintainer
#19 lore raw

PATCH v4 10/13 x86/pkeys/selftests: Fix pointer math

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-selftests-fix-pointer-math tools/testing/selftests/x86/protection_keys.c
@@ -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());

_

#20 lore raw

[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
index 55a4e349a45e..ee8176358d12 100644
@@ -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());
author maintainer
#21 lore raw

PATCH v4 11/13 x86/pkeys/selftests: Save off 'prot' for allocations

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-update-selftests-store-malloc-record tools/testing/selftests/x86/protection_keys.c
@@ -662,10 +662,12 @@ int mprotect_pkey(void *ptr, size_t size
 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;
@@ -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 siz
 	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);
@@ -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, int
 	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;
@@ -857,7 +861,7 @@ void *malloc_pkey_mmap_dax(long size, in
 
 	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);

_

#22 lore raw

[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
index ee8176358d12..986ed38a2b25 100644
@@ -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);
author maintainer
#23 lore raw

PATCH v4 12/13 x86/pkeys/selftests: Add a test for pkey 0

In reply to Dave Hansen

From: 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.

Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Shuah Khan <[email protected]>

---

 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
diff -puN tools/testing/selftests/x86/protection_keys.c~pkeys-update-selftests-with-pkey-0-test tools/testing/selftests/x86/protection_keys.c
@@ -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,

_

#24 lore raw

[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
index 986ed38a2b25..460b4bdf4c1e 100644
@@ -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,
author maintainer
#25 lore raw

PATCH v4 13/13 x86/pkeys: Do not special case protection key 0

In reply to Dave Hansen

From: 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]

Fixes 58ab9a088dda ("x86/pkeys: Check against max pkey to avoid overflows")
Cc Ram Pai <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Andrew Morton <[email protected]>p
Cc Shuah Khan <[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
diff -puN arch/x86/include/asm/mmu_context.h~x86-pkey-0-default-allocated arch/x86/include/asm/mmu_context.h
@@ -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
diff -puN arch/x86/include/asm/pkeys.h~x86-pkey-0-default-allocated arch/x86/include/asm/pkeys.h
@@ -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;

_

#26 lore raw

[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]

Cc Andrew Morton <[email protected]>p
Cc Dave Hansen <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Michael Ellermen <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ram Pai <[email protected]>
Cc Shuah Khan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Fixes 58ab9a088dda ("x86/pkeys: Check against max pkey to avoid overflows")
---
 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
index 57e3785d0d26..cf9911b5a53c 100644
@@ -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
index 39cd292805a9..851c04b7a092 100644
@@ -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;
#27 lore raw
In reply to Dave Hansen
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 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

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

#28 lore raw
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

#29 lore raw

PATCH 1/1 x86/pkeys/selftests: Adjust the self-test to fresh distros that export the pkeys ABI

In reply to Ingo Molnar

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 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
index f15aa5a76fe3..bbe80a5c31c7 100644
@@ -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());
#30 lore raw

[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
index f15aa5a76fe3..bbe80a5c31c7 100644
@@ -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());
#31 lore raw

PATCH 1/1 x86/mpx/selftests: Adjust the self-test to fresh distros that export the MPX ABI

In reply to Ingo Molnar

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
index 9c0325e1ea68..50f7e9272481 100644
@@ -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);
#32 lore raw

[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
index 9c0325e1ea68..50f7e9272481 100644
@@ -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);