Skip to contentSkip to conversation

MPX and Protection Keys Updates

PATCH In review Dave Hansen 3 participants 12 messages Nov 11, 2017 – Nov 21, 2017

Subsystems (3)
lore
Share & tools
lore

Dave Hansen

author maintainer
#0 lore raw

cover MPX and Protection Keys Updates

Hi Ingo,

Here are some small updates to Protection Keys documentation, and some small fixes to the selftests that we discussed.

author maintainer
#1 lore raw

PATCH 1/4 x86, pkeys: update documentation about availability

From: Dave Hansen <[email protected]>

Now that CPUs that implement Memory Protection Keys are publicly available we can be a bit less oblique about where it is available.

---

 b/Documentation/x86/protection-keys.txt |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Documentation/x86/protection-keys.txt renamed from Documentation/x86/protection-keys.txt~pkeys-update +7 -2
diff -puN Documentation/x86/protection-keys.txt~pkeys-update Documentation/x86/protection-keys.txt
@@ -1,5 +1,10 @@
-Memory Protection Keys for Userspace (PKU aka PKEYs) is a CPU feature
-which will be found on future Intel CPUs.
+Memory Protection Keys for Userspace (PKU aka PKEYs) is a feature
+which is found on Intel's Skylake "Scalable Processor" Server CPUs.
+It will be avalable in future non-server parts.
+
+For anyone wishing to test or use this feature, it is available in
+Amazon's EC2 C5 instances and is known to work there using an Ubuntu
+17.04 image.
 
 Memory Protection Keys provides a mechanism for enforcing page-based
 protections, but without requiring modification of the page tables

_

#2 lore raw

[tip:x86/urgent] x86/pkeys: Update documentation about availability

x86/pkeys: Update documentation about availability
Commit-ID:  c51ff2c7fc45da8b18b28c4f15eca5a9975dfb59
Gitweb:     https://git.kernel.org/tip/c51ff2c7fc45da8b18b28c4f15eca5a9975dfb59
Author:     Dave Hansen <[email protected]>
AuthorDate: Fri, 10 Nov 2017 16:12:28 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Nov 2017 09:34:52 +0100

x86/pkeys: Update documentation about availability

Now that CPUs that implement Memory Protection Keys are publicly available we can be a bit less oblique about where it is available.

Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 Documentation/x86/protection-keys.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Documentation/x86/protection-keys.txt +7 -2
index fa46dcb..ecb0d2d 100644
@@ -1,5 +1,10 @@
-Memory Protection Keys for Userspace (PKU aka PKEYs) is a CPU feature
-which will be found on future Intel CPUs.
+Memory Protection Keys for Userspace (PKU aka PKEYs) is a feature
+which is found on Intel's Skylake "Scalable Processor" Server CPUs.
+It will be avalable in future non-server parts.
+
+For anyone wishing to test or use this feature, it is available in
+Amazon's EC2 C5 instances and is known to work there using an Ubuntu
+17.04 image.
 
 Memory Protection Keys provides a mechanism for enforcing page-based
 protections, but without requiring modification of the page tables
author maintainer
#3 lore raw

PATCH 2/4 x86, selftests, mpx: fix up weird arrays

In reply to Dave Hansen

The MPX hardware data structurse are defined in a weird way: they define their size in bytes and then union that with the type with which we want to access them.

Yes, this is weird, but it does work. But, new GCC's complain that we are accessing the array out of bounds. Just make it a zero-sized array so gcc will stop complaining. There was not really a bug here.

---

 b/tools/testing/selftests/x86/mpx-hw.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/testing/selftests/x86/mpx-hw.h renamed from tools/testing/selftests/x86/mpx-hw.h~x86-selftests-mpx-weird-arrays +2 -2
diff -puN tools/testing/selftests/x86/mpx-hw.h~x86-selftests-mpx-weird-arrays tools/testing/selftests/x86/mpx-hw.h
@@ -52,14 +52,14 @@
 struct mpx_bd_entry {
 	union {
 		char x[MPX_BOUNDS_DIR_ENTRY_SIZE_BYTES];
-		void *contents[1];
+		void *contents[0];
 	};
 } __attribute__((packed));
 
 struct mpx_bt_entry {
 	union {
 		char x[MPX_BOUNDS_TABLE_ENTRY_SIZE_BYTES];
-		unsigned long contents[1];
+		unsigned long contents[0];
 	};
 } __attribute__((packed));
 

_

#4 lore raw

Re: [PATCH 2/4] x86, selftests, mpx: fix up weird arrays

Quoted 8 lines · Dave Hansen

The MPX hardware data structurse are defined in a weird way: they define their size in bytes and then union that with the type with which we want to access them.

Yes, this is weird, but it does work. But, new GCC's complain that we are accessing the array out of bounds. Just make it a zero-sized array so gcc will stop complaining. There was not really a bug here.

Note, I've added your missing Signed-off-by line.

Thanks,

Ingo

#5 lore raw

[tip:x86/urgent] x86/mpx/selftests: Fix up weird arrays

In reply to Dave Hansen
x86/mpx/selftests: Fix up weird arrays
Commit-ID:  a6400120d042397675fcf694060779d21e9e762d
Gitweb:     https://git.kernel.org/tip/a6400120d042397675fcf694060779d21e9e762d
Author:     Dave Hansen <[email protected]>
AuthorDate: Fri, 10 Nov 2017 16:12:29 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Nov 2017 09:34:52 +0100

x86/mpx/selftests: Fix up weird arrays

The MPX hardware data structurse are defined in a weird way: they define their size in bytes and then union that with the type with which we want to access them.

Yes, this is weird, but it does work. But, new GCC's complain that we are accessing the array out of bounds. Just make it a zero-sized array so gcc will stop complaining. There was not really a bug here.

Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/testing/selftests/x86/mpx-hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/testing/selftests/x86/mpx-hw.h +2 -2
index 3f00939..d1b61ab 100644
@@ -52,14 +52,14 @@
 struct mpx_bd_entry {
 	union {
 		char x[MPX_BOUNDS_DIR_ENTRY_SIZE_BYTES];
-		void *contents[1];
+		void *contents[0];
 	};
 } __attribute__((packed));
 
 struct mpx_bt_entry {
 	union {
 		char x[MPX_BOUNDS_TABLE_ENTRY_SIZE_BYTES];
-		unsigned long contents[1];
+		unsigned long contents[0];
 	};
 } __attribute__((packed));
 
author maintainer
#6 lore raw

PATCH 3/4 x86, selftests, protection_keys: rename si_pkey

In reply to Dave Hansen

si_pkey is now #defined to be the name of the new siginfo field that protection keys uses. Rename it not to conflict.

---

 b/tools/testing/selftests/x86/protection_keys.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c renamed from tools/testing/selftests/x86/protection_keys.c~x86-selftests-protection-keys-si_pkeys +5 -5
diff -puN tools/testing/selftests/x86/protection_keys.c~x86-selftests-protection-keys-si_pkeys tools/testing/selftests/x86/protection_keys.c
@@ -250,7 +250,7 @@ void signal_handler(int signum, siginfo_
 	unsigned long ip;
 	char *fpregs;
 	u32 *pkru_ptr;
-	u64 si_pkey;
+	u64 siginfo_pkey;
 	u32 *si_pkey_ptr;
 	int pkru_offset;
 	fpregset_t fpregset;
@@ -292,9 +292,9 @@ void signal_handler(int signum, siginfo_
 	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);
-	si_pkey = *si_pkey_ptr;
-	pkey_assert(si_pkey < NR_PKEYS);
-	last_si_pkey = si_pkey;
+	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) ||
@@ -306,7 +306,7 @@ void signal_handler(int signum, siginfo_
 	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());
-	dprintf1("si_pkey from siginfo: %jx\n", si_pkey);
+	dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
 	*(u64 *)pkru_ptr = 0x00000000;
 	dprintf1("WARNING: set PRKU=0 to allow faulting instruction to continue\n");
 	pkru_faults++;

_

#7 lore raw

Re: [PATCH 3/4] x86, selftests, protection_keys: rename si_pkey

Quoted 5 lines · Dave Hansen

si_pkey is now #defined to be the name of the new siginfo field that protection keys uses. Rename it not to conflict.

---

Ditto: added your Signed-off-by which I presume was intended.

Thanks,

Ingo

#8 lore raw

[tip:x86/urgent] x86/pkeys/selftests: Rename 'si_pkey' to 'siginfo_pkey'

In reply to Dave Hansen
x86/pkeys/selftests: Rename 'si_pkey' to 'siginfo_pkey'
Commit-ID:  91c49c2deb96ffc3c461eaae70219d89224076b7
Gitweb:     https://git.kernel.org/tip/91c49c2deb96ffc3c461eaae70219d89224076b7
Author:     Dave Hansen <[email protected]>
AuthorDate: Fri, 10 Nov 2017 16:12:31 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Nov 2017 09:34:52 +0100

x86/pkeys/selftests: Rename 'si_pkey' to 'siginfo_pkey'

'si_pkey' is now #defined to be the name of the new siginfo field that protection keys uses. Rename it not to conflict.

Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/testing/selftests/x86/protection_keys.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/testing/selftests/x86/protection_keys.c +5 -5
index 7a1cc0e..bc1b073 100644
@@ -250,7 +250,7 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
 	unsigned long ip;
 	char *fpregs;
 	u32 *pkru_ptr;
-	u64 si_pkey;
+	u64 siginfo_pkey;
 	u32 *si_pkey_ptr;
 	int pkru_offset;
 	fpregset_t fpregset;
@@ -292,9 +292,9 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
 	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);
-	si_pkey = *si_pkey_ptr;
-	pkey_assert(si_pkey < NR_PKEYS);
-	last_si_pkey = si_pkey;
+	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) ||
@@ -306,7 +306,7 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)
 	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());
-	dprintf1("si_pkey from siginfo: %jx\n", si_pkey);
+	dprintf1("pkey from siginfo: %jx\n", siginfo_pkey);
 	*(u64 *)pkru_ptr = 0x00000000;
 	dprintf1("WARNING: set PRKU=0 to allow faulting instruction to continue\n");
 	pkru_faults++;
author maintainer
#9 lore raw

PATCH 4/4 x86, selftests: fix protection keys write() warining

In reply to Dave Hansen

From: Dave Hansen <[email protected]>

write() is marked as having a must-check return value. Check it and abort if we fail to write an error message from a signal handler.

---

 b/tools/testing/selftests/x86/pkey-helpers.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/pkey-helpers.h renamed from tools/testing/selftests/x86/pkey-helpers.h~x86-selftests-protection-keys-write-warning +4 -1
diff -puN tools/testing/selftests/x86/pkey-helpers.h~x86-selftests-protection-keys-write-warning tools/testing/selftests/x86/pkey-helpers.h
@@ -30,6 +30,7 @@ static inline void sigsafe_printf(const
 	if (!dprint_in_signal) {
 		vprintf(format, ap);
 	} else {
+		int ret;
 		int len = vsnprintf(dprint_in_signal_buffer,
 				    DPRINT_IN_SIGNAL_BUF_SIZE,
 				    format, ap);
@@ -39,7 +40,9 @@ static inline void sigsafe_printf(const
 		 */
 		if (len > DPRINT_IN_SIGNAL_BUF_SIZE)
 			len = DPRINT_IN_SIGNAL_BUF_SIZE;
-		write(1, dprint_in_signal_buffer, len);
+		ret = write(1, dprint_in_signal_buffer, len);
+		if (ret < 0)
+			abort();
 	}
 	va_end(ap);
 }

_

#10 lore raw

[tip:x86/urgent] x86/pkeys/selftests: Fix protection keys write() warning

x86/pkeys/selftests: Fix protection keys write() warning
Commit-ID:  7b659ee3e1fe0e8eb39730afb903c64e25490ec4
Gitweb:     https://git.kernel.org/tip/7b659ee3e1fe0e8eb39730afb903c64e25490ec4
Author:     Dave Hansen <[email protected]>
AuthorDate: Fri, 10 Nov 2017 16:12:32 -0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Nov 2017 09:34:52 +0100

x86/pkeys/selftests: Fix protection keys write() warning

write() is marked as having a must-check return value. Check it and abort if we fail to write an error message from a signal handler.

Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/testing/selftests/x86/pkey-helpers.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/testing/selftests/x86/pkey-helpers.h +4 -1
index 3818f25..b3cb767 100644
@@ -30,6 +30,7 @@ static inline void sigsafe_printf(const char *format, ...)
 	if (!dprint_in_signal) {
 		vprintf(format, ap);
 	} else {
+		int ret;
 		int len = vsnprintf(dprint_in_signal_buffer,
 				    DPRINT_IN_SIGNAL_BUF_SIZE,
 				    format, ap);
@@ -39,7 +40,9 @@ static inline void sigsafe_printf(const char *format, ...)
 		 */
 		if (len > DPRINT_IN_SIGNAL_BUF_SIZE)
 			len = DPRINT_IN_SIGNAL_BUF_SIZE;
-		write(1, dprint_in_signal_buffer, len);
+		ret = write(1, dprint_in_signal_buffer, len);
+		if (ret < 0)
+			abort();
 	}
 	va_end(ap);
 }
#11 lore raw
In reply to Dave Hansen
Quoted 4 lines · Dave Hansen

Hi Ingo,

Here are some small updates to Protection Keys documentation, and some small fixes to the selftests that we discussed.

Note that even with all the patches applied, a build warning remains:

gcc -m32 -o /home/mingo/tip/tools/testing/selftests/x86/protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall -no-pie protection_keys.c -lrt -ldl -lm protection_keys.c: In function ‘dumpit’:
protection_keys.c:419:3: warning: ignoring return value of ‘write’, declared with
attribute warn_unused_result [-Wunused-result]
write(1, buf, nr_read);
^~~~~~~~~~~~~~~~~~~~~~

Thanks,

Ingo