[PATCH 0/4] MPX and Protection Keys Updates

[PATCH 0/4] MPX and Protection Keys Updates

[Dave Hansen]

Hi Ingo,

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

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

[Dave Hansen]

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

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

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/Documentation/x86/protection-keys.txt |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff -puN Documentation/x86/protection-keys.txt~pkeys-update Documentation/x86/protection-keys.txt
--- a/Documentation/x86/protection-keys.txt~pkeys-update	2017-11-10 15:29:24.314207951 -0800
+++ b/Documentation/x86/protection-keys.txt	2017-11-10 15:29:24.317207951 -0800
@@ -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
_

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

[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(-)

diff -puN tools/testing/selftests/x86/mpx-hw.h~x86-selftests-mpx-weird-arrays tools/testing/selftests/x86/mpx-hw.h
--- a/tools/testing/selftests/x86/mpx-hw.h~x86-selftests-mpx-weird-arrays	2017-11-10 15:29:24.846207949 -0800
+++ b/tools/testing/selftests/x86/mpx-hw.h	2017-11-10 15:29:24.850207949 -0800
@@ -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));
 
_

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

[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(-)

diff -puN tools/testing/selftests/x86/protection_keys.c~x86-selftests-protection-keys-si_pkeys tools/testing/selftests/x86/protection_keys.c
--- a/tools/testing/selftests/x86/protection_keys.c~x86-selftests-protection-keys-si_pkeys	2017-11-10 15:29:25.380207948 -0800
+++ b/tools/testing/selftests/x86/protection_keys.c	2017-11-10 15:29:25.384207948 -0800
@@ -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++;
_

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

[Dave Hansen]

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

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.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/tools/testing/selftests/x86/pkey-helpers.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff -puN tools/testing/selftests/x86/pkey-helpers.h~x86-selftests-protection-keys-write-warning tools/testing/selftests/x86/pkey-helpers.h
--- a/tools/testing/selftests/x86/pkey-helpers.h~x86-selftests-protection-keys-write-warning	2017-11-10 15:29:29.240207938 -0800
+++ b/tools/testing/selftests/x86/pkey-helpers.h	2017-11-10 15:29:29.244207938 -0800
@@ -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);
 }
_

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

[Ingo Molnar]

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

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

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

[Ingo Molnar]

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

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

Re: [PATCH 0/4] MPX and Protection Keys Updates

[Ingo Molnar]

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

> 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

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

[tip-bot for Dave Hansen]

Commit-ID:  c51ff2c7fc45da8b18b28c4f15eca5a9975dfb59
Gitweb:     https://git.kernel.org/tip/c51ff2c7fc45da8b18b28c4f15eca5a9975dfb59
Author:     Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Fri, 10 Nov 2017 16:12:28 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20171111001228.DC748A10@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 Documentation/x86/protection-keys.txt | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/x86/protection-keys.txt b/Documentation/x86/protection-keys.txt
index fa46dcb..ecb0d2d 100644
--- a/Documentation/x86/protection-keys.txt
+++ b/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

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

[tip-bot for Dave Hansen]

Commit-ID:  a6400120d042397675fcf694060779d21e9e762d
Gitweb:     https://git.kernel.org/tip/a6400120d042397675fcf694060779d21e9e762d
Author:     Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Fri, 10 Nov 2017 16:12:29 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20171111001229.58A7933D@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/x86/mpx-hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/x86/mpx-hw.h b/tools/testing/selftests/x86/mpx-hw.h
index 3f00939..d1b61ab 100644
--- a/tools/testing/selftests/x86/mpx-hw.h
+++ b/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));
 

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

[tip-bot for Dave Hansen]

Commit-ID:  91c49c2deb96ffc3c461eaae70219d89224076b7
Gitweb:     https://git.kernel.org/tip/91c49c2deb96ffc3c461eaae70219d89224076b7
Author:     Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Fri, 10 Nov 2017 16:12:31 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20171111001231.DFFC8285@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/x86/protection_keys.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/x86/protection_keys.c b/tools/testing/selftests/x86/protection_keys.c
index 7a1cc0e..bc1b073 100644
--- a/tools/testing/selftests/x86/protection_keys.c
+++ b/tools/testing/selftests/x86/protection_keys.c
@@ -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++;

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

[tip-bot for Dave Hansen]

Commit-ID:  7b659ee3e1fe0e8eb39730afb903c64e25490ec4
Gitweb:     https://git.kernel.org/tip/7b659ee3e1fe0e8eb39730afb903c64e25490ec4
Author:     Dave Hansen <dave.hansen@linux.intel.com>
AuthorDate: Fri, 10 Nov 2017 16:12:32 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20171111001232.94813E58@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/x86/pkey-helpers.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/pkey-helpers.h b/tools/testing/selftests/x86/pkey-helpers.h
index 3818f25..b3cb767 100644
--- a/tools/testing/selftests/x86/pkey-helpers.h
+++ b/tools/testing/selftests/x86/pkey-helpers.h
@@ -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);
 }