selftests, powerpc, x86 : Memory Protection Keys
Subsystems (4)
v13 cover selftests, powerpc, x86 : Memory Protection Keys
Memory protection keys enables an application to protect its address space from inadvertent access by its own code.
This feature is now enabled on powerpc architecture and integrated in 4.16-rc1. The patches move the selftests to arch neutral directory and enhance their test coverage.
Test
----
Verified for correctness on powerpc. Need help verifying on x86. Compiles on x86.
History:
-------
version 13:
(1) Incorporated comments for Dave Hansen.
(2) Added one more test for correct pkey-0 behavior.
version 12:
(1) fixed the offset of pkey field in the siginfo structure for x86_64 and powerpc. And tries to use the actual field if the headers have it defined.
version 11:
(1) fixed a deadlock in the ptrace testcase.
version 10 and prior:
(1) moved the testcase to arch neutral directory
(2) split the changes into incremental patches.
Ram Pai (22):
selftests/x86: Move protecton key selftest to arch neutral directory selftests/vm: rename all references to pkru to a generic name selftests/vm: move generic definitions to header file selftests/vm: typecast the pkey register
selftests/vm: generic function to handle shadow key register selftests/vm: fix the wrong assert in pkey_disable_set() selftests/vm: fixed bugs in pkey_disable_clear() selftests/vm: clear the bits in shadow reg when a pkey is freed. selftests/vm: fix alloc_random_pkey() to make it really random selftests/vm: introduce two arch independent abstraction selftests/vm: pkey register should match shadow pkey selftests/vm: generic cleanup
selftests/vm: powerpc implementation for generic abstraction selftests/vm: clear the bits in shadow reg when a pkey is freed. selftests/vm: powerpc implementation to check support for pkey selftests/vm: fix an assertion in test_pkey_alloc_exhaust() selftests/vm: associate key on a mapped page and detect access
violation
selftests/vm: associate key on a mapped page and detect write
violation
selftests/vm: detect write violation on a mapped access-denied-key
page
selftests/vm: testcases must restore pkey-permissions selftests/vm: sub-page allocator
selftests/vm: test correct behavior of pkey-0
Thiago Jung Bauermann (2):
selftests/vm: move arch-specific definitions to arch-specific header selftests/vm: Make gcc check arguments of sigsafe_printf()
tools/testing/selftests/vm/.gitignore | 1 + tools/testing/selftests/vm/Makefile | 1 + tools/testing/selftests/vm/pkey-helpers.h | 214 ++++ tools/testing/selftests/vm/pkey-powerpc.h | 126 ++ tools/testing/selftests/vm/pkey-x86.h | 184 +++ tools/testing/selftests/vm/protection_keys.c | 1598 +++++++++++++++++++++++++ tools/testing/selftests/x86/.gitignore | 1 - tools/testing/selftests/x86/pkey-helpers.h | 219 ---- tools/testing/selftests/x86/protection_keys.c | 1485 ----------------------- 9 files changed, 2124 insertions(+), 1705 deletions(-) create mode 100644 tools/testing/selftests/vm/pkey-helpers.h create mode 100644 tools/testing/selftests/vm/pkey-powerpc.h create mode 100644 tools/testing/selftests/vm/pkey-x86.h create mode 100644 tools/testing/selftests/vm/protection_keys.c delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h delete mode 100644 tools/testing/selftests/x86/protection_keys.c
PATCH v13 1/24 selftests/x86: Move protecton key selftest to arch neutral directory
--- tools/testing/selftests/vm/.gitignore | 1 + tools/testing/selftests/vm/Makefile | 1 + tools/testing/selftests/vm/pkey-helpers.h | 219 ++++ tools/testing/selftests/vm/protection_keys.c | 1485 +++++++++++++++++++++++++ tools/testing/selftests/x86/.gitignore | 1 - tools/testing/selftests/x86/pkey-helpers.h | 219 ---- tools/testing/selftests/x86/protection_keys.c | 1485 ------------------------- 7 files changed, 1706 insertions(+), 1705 deletions(-) create mode 100644 tools/testing/selftests/vm/pkey-helpers.h create mode 100644 tools/testing/selftests/vm/protection_keys.c delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h delete mode 100644 tools/testing/selftests/x86/protection_keys.c
tools/testing/selftests/vm/.gitignore +1 -0
@@ -12,3 +12,4 @@ mlock-random-test virtual_address_range gup_benchmark va_128TBswitch +protection_keys
tools/testing/selftests/vm/Makefile +1 -0
@@ -20,6 +20,7 @@ TEST_GEN_FILES += transhuge-stress TEST_GEN_FILES += userfaultfd TEST_GEN_FILES += va_128TBswitch TEST_GEN_FILES += virtual_address_range +TEST_GEN_FILES += protection_keys TEST_PROGS := run_vmtests
tools/testing/selftests/vm/pkey-helpers.h new file +219 -0
219 lines not on this page · raw message
tools/testing/selftests/vm/protection_keys.c new file +1485 -0
1,485 lines not on this page · raw message
tools/testing/selftests/x86/.gitignore +0 -1
@@ -11,5 +11,4 @@ ldt_gdt iopl mpx-mini-test ioperm -protection_keys test_vdso
tools/testing/selftests/x86/pkey-helpers.h deleted +0 -219
219 lines not on this page · raw message
tools/testing/selftests/x86/protection_keys.c deleted +0 -1485
1,061 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
1.7.1
Part of the diff is not shown here. View the raw message.
PATCH v13 2/24 selftests/vm: rename all references to pkru to a generic name
In reply to Ram Paisome pkru references are named to pkey_reg
and some prku references are renamed to pkey
--- tools/testing/selftests/vm/pkey-helpers.h | 85 +++++----- tools/testing/selftests/vm/protection_keys.c | 238 ++++++++++++++------------ 2 files changed, 169 insertions(+), 154 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +44 -41
@@ -14,7 +14,7 @@#include <sys/mman.h> #define NR_PKEYS 16 -#define PKRU_BITS_PER_PKEY 2 +#define PKEY_BITS_PER_PKEY 2#ifndef DEBUG_LEVEL #define DEBUG_LEVEL 0
@@ -53,85 +53,88 @@ static inline void sigsafe_printf(const char *format, ...) #define dprintf3(args...) dprintf_level(3, args) #define dprintf4(args...) dprintf_level(4, args) -extern unsigned int shadow_pkru; -static inline unsigned int __rdpkru (void ) +extern unsigned int shadow_pkey_reg; +static inline unsigned int __read_pkey_reg (void ) {unsigned int eax, edx;unsigned int ecx =0 ; -unsigned int pkru; +unsigned int pkey_reg; asmvolatile (".byte 0x0f,0x01,0xee\n\t" :"=a" (eax),"=d" (edx) :"c" (ecx)); - pkru = eax; -return pkru; + pkey_reg = eax; +return pkey_reg; } -static inline unsigned int _rdpkru (int line) +static inline unsigned int _read_pkey_reg (int line) { -unsigned int pkru =__rdpkru (); +unsigned int pkey_reg =__read_pkey_reg (); -dprintf4 ("rdpkru(line=%d) pkru: %x shadow: %x\n" , - line, pkru, shadow_pkru); -assert (pkru == shadow_pkru); +dprintf4 ("read_pkey_reg(line=%d) pkey_reg: %x shadow: %x\n" , + line, pkey_reg, shadow_pkey_reg); +assert (pkey_reg == shadow_pkey_reg); -return pkru; +return pkey_reg; } -#define rdpkru () _rdpkru(__LINE__) +#define read_pkey_reg () _read_pkey_reg(__LINE__) -static inline void __wrpkru (unsigned int pkru) +static inline void __write_pkey_reg (unsigned int pkey_reg) { -unsigned int eax = pkru; +unsigned int eax = pkey_reg;unsigned int ecx =0 ;unsigned int edx =0 ; -dprintf4 ("%s() changing %08x to %08x\n" , __func__,__rdpkru (), pkru); +dprintf4 ("%s() changing %08x to %08x\n" , __func__, +__read_pkey_reg (), pkey_reg); asmvolatile (".byte 0x0f,0x01,0xef\n\t" : :"a" (eax),"c" (ecx),"d" (edx)); -assert (pkru ==__rdpkru ()); +assert (pkey_reg ==__read_pkey_reg ()); } -static inline void wrpkru (unsigned int pkru) +static inline void write_pkey_reg (unsigned int pkey_reg) { -dprintf4 ("%s() changing %08x to %08x\n" , __func__,__rdpkru (), pkru); +dprintf4 ("%s() changing %08x to %08x\n" , __func__, +__read_pkey_reg (), pkey_reg);/* will do the shadow check for us: */ -rdpkru (); -__wrpkru (pkru); - shadow_pkru = pkru; -dprintf4 ("%s(%08x) pkru: %08x\n" , __func__, pkru,__rdpkru ()); +read_pkey_reg (); +__write_pkey_reg (pkey_reg); + shadow_pkey_reg = pkey_reg; +dprintf4 ("%s(%08x) pkey_reg: %08x\n" , __func__, + pkey_reg,__read_pkey_reg ()); }/* * These are technically racy. since something could -* change PKRU between the read and the write. +* change PKEY register between the read and the write. */ static inline void __pkey_access_allow (int pkey,int do_allow) { -unsigned int pkru =rdpkru (); +unsigned int pkey_reg =read_pkey_reg ();int bit = pkey *2 ;if (do_allow) - pkru &= (1 <<bit); + pkey_reg &= (1 <<bit);else - pkru |= (1 <<bit); + pkey_reg |= (1 <<bit); -dprintf4 ("pkru now: %08x\n" ,rdpkru ()); -wrpkru (pkru); +dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); +write_pkey_reg (pkey_reg); }static inline void __pkey_write_allow (int pkey,int do_allow_write) { -long pkru =rdpkru (); +long pkey_reg =read_pkey_reg ();int bit = pkey *2 +1 ;if (do_allow_write) - pkru &= (1 <<bit); + pkey_reg &= (1 <<bit);else - pkru |= (1 <<bit); + pkey_reg |= (1 <<bit); -wrpkru (pkru); -dprintf4 ("pkru now: %08x\n" ,rdpkru ()); +write_pkey_reg (pkey_reg); +dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); }#define PROT_PKEY0 0x10/* protection key value (bit 0) */
@@ -181,10 +184,10 @@ static inline int cpu_has_pku(void)return 1 ; } -#define XSTATE_PKRU_BIT (9) -#define XSTATE_PKRU 0x200 +#define XSTATE_PKEY_BIT (9) +#define XSTATE_PKEY 0x200 -int pkru_xstate_offset (void ) +int pkey_reg_xstate_offset (void ) {unsigned int eax;unsigned int ebx;
@@ -195,21 +198,21 @@ int pkru_xstate_offset(void)unsigned long XSTATE_CPUID =0xd ;int leaf; -/* assume that XSTATE_PKRU is set in XCR0 */ - leaf =XSTATE_PKRU_BIT ; +/* assume that XSTATE_PKEY is set in XCR0 */ + leaf =XSTATE_PKEY_BIT ; { eax =XSTATE_CPUID ; ecx = leaf;__cpuid (&eax, &ebx, &ecx, &edx); -if (leaf ==XSTATE_PKRU_BIT ) { +if (leaf ==XSTATE_PKEY_BIT ) { xstate_offset = ebx; xstate_size = eax; } }if (xstate_size ==0 ) { -printf ("could not find size/offset of PKRU in xsave state\n" ); +printf ("could not find size/offset of PKEY in xsave state\n" );return 0 ; }
tools/testing/selftests/vm/protection_keys.c +125 -113
527 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH v13 3/24 selftests/vm: move generic definitions to header file
In reply to Ram PaiMoved all the generic definition and helper functions to the header file.
--- tools/testing/selftests/vm/pkey-helpers.h | 35 ++++++++++++++++++++++--- tools/testing/selftests/vm/protection_keys.c | 27 -------------------- 2 files changed, 30 insertions(+), 32 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +30 -5
@@ -13,6 +13,14 @@#include <ucontext.h> #include <sys/mman.h> +/* Define some kernel-like types */ +#define u8 uint8_t +#define u16 uint16_t +#define u32 uint32_t +#define u64 uint64_t + +#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP) +#define NR_PKEYS 16#define PKEY_BITS_PER_PKEY 2
@@ -53,6 +61,18 @@ static inline void sigsafe_printf(const char *format, ...) #define dprintf3(args...) dprintf_level(3, args) #define dprintf4(args...) dprintf_level(4, args) +extern void abort_hooks (void ); +#define pkey_assert (condition) do { \ + if (!(condition)) { \ + dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \ + __FILE__, __LINE__, \ + test_nr, iteration_nr); \ + dprintf0("errno at assert: %d", errno); \ + abort_hooks(); \ + exit(__LINE__); \ + } \ +} while (0) +extern unsigned int shadow_pkey_reg;static inline unsigned int __read_pkey_reg (void ) {
@@ -137,11 +157,6 @@ static inline void __pkey_write_allow(int pkey, int do_allow_write)dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); } -#define PROT_PKEY0 0x10/* protection key value (bit 0) */ -#define PROT_PKEY1 0x20/* protection key value (bit 1) */ -#define PROT_PKEY2 0x40/* protection key value (bit 2) */ -#define PROT_PKEY3 0x80/* protection key value (bit 3) */ -#define PAGE_SIZE 4096#define MB (1<<20)
@@ -219,4 +234,14 @@ int pkey_reg_xstate_offset(void)return xstate_offset; } +#define ARRAY_SIZE (x) (sizeof(x) / sizeof(*(x))) +#define ALIGN_UP (x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1)) +#define ALIGN_DOWN (x, align_to) ((x) & ~((align_to)-1)) +#define ALIGN_PTR_UP (p, ptr_align_to) \ + ((typeof(p))ALIGN_UP((unsigned long)(p), ptr_align_to)) +#define ALIGN_PTR_DOWN (p, ptr_align_to) \ + ((typeof(p))ALIGN_DOWN((unsigned long)(p), ptr_align_to)) +#define __stringify_1(x...) #x +#define __stringify(x...) __stringify_1(x) +#endif /* _PKEYS_HELPER_H */
tools/testing/selftests/vm/protection_keys.c +0 -27
@@ -51,31 +51,10 @@unsigned int shadow_pkey_reg;#define HPAGE_SIZE (1UL<<21) -#define ARRAY_SIZE (x) (sizeof(x) / sizeof(*(x))) -#define ALIGN_UP (x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1)) -#define ALIGN_DOWN (x, align_to) ((x) & ~((align_to)-1)) -#define ALIGN_PTR_UP (p, ptr_align_to) ((typeof(p))ALIGN_UP((unsigned long)(p), ptr_align_to)) -#define ALIGN_PTR_DOWN (p, ptr_align_to) ((typeof(p))ALIGN_DOWN((unsigned long)(p), ptr_align_to)) -#define __stringify_1(x...) #x -#define __stringify(x...) __stringify_1(x) - -#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP)int dprint_in_signal;char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE ]; -extern void abort_hooks (void ); -#define pkey_assert (condition) do { \ - if (!(condition)) { \ - dprintf0("assert() at %s::%d test_nr: %d iteration: %d\n", \ - __FILE__, __LINE__, \ - test_nr, iteration_nr); \ - dprintf0("errno at assert: %d", errno); \ - abort_hooks(); \ - exit(__LINE__); \ - } \ -} while (0) -void cat_into_file (char *str,char *file) {int fd =open (file,O_RDWR );
@@ -186,12 +165,6 @@ void lots_o_noops_around_write(int *write_to_me)dprintf3 ("%s() done\n" , __func__); } -/* Define some kernel-like types */ -#define u8 uint8_t -#define u16 uint16_t -#define u32 uint32_t -#define u64 uint64_t -#ifdef __i386__#ifndef SYS_mprotect_key
Signature · 1 line
1.7.1
PATCH v13 4/24 selftests/vm: move arch-specific definitions to arch-specific header
In reply to Ram PaiFrom: Thiago Jung Bauermann <[email protected]>
In preparation for multi-arch support, move definitions which have arch-specific values to x86-specific header.
--- tools/testing/selftests/vm/pkey-helpers.h | 111 +----------------- tools/testing/selftests/vm/pkey-x86.h | 156 ++++++++++++++++++++++++++ tools/testing/selftests/vm/protection_keys.c | 47 -------- 3 files changed, 162 insertions(+), 152 deletions(-) create mode 100644 tools/testing/selftests/vm/pkey-x86.h
tools/testing/selftests/vm/pkey-helpers.h +6 -105
@@ -21,9 +21,6 @@#define PTR_ERR_ENOTSUP ((void *)-ENOTSUP) -#define NR_PKEYS 16 -#define PKEY_BITS_PER_PKEY 2 -#ifndef DEBUG_LEVEL #define DEBUG_LEVEL 0#endif
@@ -73,19 +70,13 @@ static inline void sigsafe_printf(const char *format, ...) } \ } while (0) +#if defined(__i386__) || defined(__x86_64__)/* arch */ +#include "pkey-x86.h" +#else /* arch */ +#error Architecture not supported +#endif /* arch */ +extern unsigned int shadow_pkey_reg; -static inline unsigned int __read_pkey_reg (void ) -{ -unsigned int eax, edx; -unsigned int ecx =0 ; -unsigned int pkey_reg; - - asmvolatile (".byte 0x0f,0x01,0xee\n\t" - :"=a" (eax),"=d" (edx) - :"c" (ecx)); - pkey_reg = eax; -return pkey_reg; -}static inline unsigned int _read_pkey_reg (int line) {
@@ -100,19 +91,6 @@ static inline unsigned int _read_pkey_reg(int line)#define read_pkey_reg () _read_pkey_reg(__LINE__) -static inline void __write_pkey_reg (unsigned int pkey_reg) -{ -unsigned int eax = pkey_reg; -unsigned int ecx =0 ; -unsigned int edx =0 ; - -dprintf4 ("%s() changing %08x to %08x\n" , __func__, -__read_pkey_reg (), pkey_reg); - asmvolatile (".byte 0x0f,0x01,0xef\n\t" - : :"a" (eax),"c" (ecx),"d" (edx)); -assert (pkey_reg ==__read_pkey_reg ()); -} -static inline void write_pkey_reg (unsigned int pkey_reg) { dprintf4("%s() changing %08x to %08x\n", __func__,
@@ -157,83 +135,6 @@ static inline void __pkey_write_allow(int pkey, int do_allow_write)dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); } -#define PAGE_SIZE 4096 -#define MB (1<<20) - -static inline void __cpuid (unsigned int *eax,unsigned int *ebx, -unsigned int *ecx,unsigned int *edx) -{ -/* ecx is often an input as well as an output. */ - asmvolatile ( -"cpuid;" - :"=a" (*eax), -"=b" (*ebx), -"=c" (*ecx), -"=d" (*edx) - :"0" (*eax),"2" (*ecx)); -} - -/* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx) */ -#define X86_FEATURE_PKU (1<<3)/* Protection Keys for Userspace */ -#define X86_FEATURE_OSPKE (1<<4)/* OS Protection Keys Enable */ - -static inline int cpu_has_pku (void ) -{ -unsigned int eax; -unsigned int ebx; -unsigned int ecx; -unsigned int edx; - - eax =0x7 ; - ecx =0x0 ; -__cpuid (&eax, &ebx, &ecx, &edx); - -if (!(ecx &X86_FEATURE_PKU )) { -dprintf2 ("cpu does not have PKU\n" ); -return 0 ; - } -if (!(ecx &X86_FEATURE_OSPKE )) { -dprintf2 ("cpu does not have OSPKE\n" ); -return 0 ; - } -return 1 ; -} - -#define XSTATE_PKEY_BIT (9) -#define XSTATE_PKEY 0x200 - -int pkey_reg_xstate_offset (void ) -{ -unsigned int eax; -unsigned int ebx; -unsigned int ecx; -unsigned int edx; -int xstate_offset; -int xstate_size; -unsigned long XSTATE_CPUID =0xd ; -int leaf; - -/* assume that XSTATE_PKEY is set in XCR0 */ - leaf =XSTATE_PKEY_BIT ; - { - eax =XSTATE_CPUID ; - ecx = leaf; -__cpuid (&eax, &ebx, &ecx, &edx); - -if (leaf ==XSTATE_PKEY_BIT ) { - xstate_offset = ebx; - xstate_size = eax; - } - } - -if (xstate_size ==0 ) { -printf ("could not find size/offset of PKEY in xsave state\n" ); -return 0 ; - } - -return xstate_offset; -} -#define ARRAY_SIZE (x) (sizeof(x) / sizeof(*(x)))#define ALIGN_UP (x, align_to) (((x) + ((align_to)-1)) & ~((align_to)-1))#define ALIGN_DOWN (x, align_to) ((x) & ~((align_to)-1))
tools/testing/selftests/vm/pkey-x86.h new file +156 -0
156 lines not on this page · raw message
tools/testing/selftests/vm/protection_keys.c +0 -47
71 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH v13 5/24 selftests/vm: Make gcc check arguments of sigsafe_printf()
In reply to Ram PaiFrom: Thiago Jung Bauermann <[email protected]>
This will help us ensure we print pkey_reg_t values correctly in different architectures.
--- tools/testing/selftests/vm/pkey-helpers.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +4 -0
@@ -27,6 +27,10 @@#define DPRINT_IN_SIGNAL_BUF_SIZE 4096extern int dprint_in_signal;extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE ]; + +#ifdef __GNUC__ +__attribute__((format(printf, 1, 2))) +#endif static inline void sigsafe_printf (const char *format, ...) {va_list ap;
Signature · 1 line
1.7.1
PATCH v13 6/24 selftests/vm: typecast the pkey register
In reply to Ram PaiThis is in preparation to accomadate a differing size register across architectures.
--- tools/testing/selftests/vm/pkey-helpers.h | 23 ++++--- tools/testing/selftests/vm/pkey-x86.h | 16 +++-- tools/testing/selftests/vm/protection_keys.c | 87 +++++++++++++++---------- 3 files changed, 73 insertions(+), 53 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +12 -11
@@ -80,13 +80,14 @@ static inline void sigsafe_printf(const char *format, ...)#error Architecture not supported#endif /* arch */ -extern unsigned int shadow_pkey_reg; +extern pkey_reg_t shadow_pkey_reg; -static inline unsigned int _read_pkey_reg (int line) +static inline pkey_reg_t _read_pkey_reg (int line) { -unsigned int pkey_reg =__read_pkey_reg (); +pkey_reg_t pkey_reg =__read_pkey_reg (); -dprintf4 ("read_pkey_reg(line=%d) pkey_reg: %x shadow: %x\n" , +dprintf4 ("read_pkey_reg(line=%d) pkey_reg: " PKEY_REG_FMT +" shadow: " PKEY_REG_FMT "\n" , line, pkey_reg, shadow_pkey_reg);assert (pkey_reg == shadow_pkey_reg);
@@ -95,15 +96,15 @@ static inline unsigned int _read_pkey_reg(int line)#define read_pkey_reg () _read_pkey_reg(__LINE__) -static inline void write_pkey_reg (unsigned int pkey_reg) +static inline void write_pkey_reg (pkey_reg_t pkey_reg) { -dprintf4 ("%s() changing %08x to %08x\n" , __func__, +dprintf4 ("%s() changing " PKEY_REG_FMT " to " PKEY_REG_FMT "\n" , __func__,__read_pkey_reg (), pkey_reg);/* will do the shadow check for us: */ read_pkey_reg ();__write_pkey_reg (pkey_reg); shadow_pkey_reg = pkey_reg; -dprintf4 ("%s(%08x) pkey_reg: %08x\n" , __func__, +dprintf4 ("%s(" PKEY_REG_FMT ") pkey_reg: " PKEY_REG_FMT "\n" , __func__, pkey_reg,__read_pkey_reg ()); }
@@ -113,7 +114,7 @@ static inline void write_pkey_reg(unsigned int pkey_reg)*/ static inline void __pkey_access_allow (int pkey,int do_allow) { -unsigned int pkey_reg =read_pkey_reg (); +pkey_reg_t pkey_reg =read_pkey_reg ();int bit = pkey *2 ;if (do_allow)
@@ -121,13 +122,13 @@ static inline void __pkey_access_allow(int pkey, int do_allow)else pkey_reg |= (1 <<bit); -dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); +dprintf4 ("pkey_reg now: " PKEY_REG_FMT "\n" ,read_pkey_reg ());write_pkey_reg (pkey_reg); }static inline void __pkey_write_allow (int pkey,int do_allow_write) { -long pkey_reg =read_pkey_reg (); +pkey_reg_t pkey_reg =read_pkey_reg ();int bit = pkey *2 +1 ;if (do_allow_write)
@@ -136,7 +137,7 @@ static inline void __pkey_write_allow(int pkey, int do_allow_write) pkey_reg |= (1 <<bit);write_pkey_reg (pkey_reg); -dprintf4 ("pkey_reg now: %08x\n" ,read_pkey_reg ()); +dprintf4 ("pkey_reg now: " PKEY_REG_FMT "\n" ,read_pkey_reg ()); }#define ARRAY_SIZE (x) (sizeof(x) / sizeof(*(x)))
tools/testing/selftests/vm/pkey-x86.h +9 -7
@@ -46,6 +46,8 @@#define HPAGE_SIZE (1UL<<21)#define PAGE_SIZE 4096#define MB (1<<20) +#define pkey_reg_t u32 +#define PKEY_REG_FMT "%016x"static inline void __page_o_noops (void ) {
@@ -53,11 +55,11 @@ static inline void __page_o_noops(void) asm(".rept 512 ; nopl 0x7eeeeeee(%eax) ; .endr" ); } -static inline unsigned int __read_pkey_reg (void ) +static inline pkey_reg_t __read_pkey_reg (void ) {unsigned int eax, edx;unsigned int ecx =0 ; -unsigned int pkey_reg; +pkey_reg_t pkey_reg; asmvolatile (".byte 0x0f,0x01,0xee\n\t" : "=a" (eax), "=d" (edx)
@@ -66,13 +68,13 @@ static inline unsigned int __read_pkey_reg(void)return pkey_reg; } -static inline void __write_pkey_reg (unsigned int pkey_reg) +static inline void __write_pkey_reg (pkey_reg_t pkey_reg) { -unsigned int eax = pkey_reg; -unsigned int ecx =0 ; -unsigned int edx =0 ; +pkey_reg_t eax = pkey_reg; +pkey_reg_t ecx =0 ; +pkey_reg_t edx =0 ; -dprintf4 ("%s() changing %08x to %08x\n" , __func__, +dprintf4 ("%s() changing " PKEY_REG_FMT " to " PKEY_REG_FMT "\n" , __func__,__read_pkey_reg (), pkey_reg); asmvolatile (".byte 0x0f,0x01,0xef\n\t" : :"a" (eax),"c" (ecx),"d" (edx));
tools/testing/selftests/vm/protection_keys.c +52 -35
265 lines not on this page · raw message
Signature · 1 line
1.7.1
PATCH v13 7/24 selftests/vm: generic function to handle shadow key register
In reply to Ram Paihelper functions to handler shadow pkey register
--- tools/testing/selftests/vm/pkey-helpers.h | 29 ++++++++++++++++++++++ tools/testing/selftests/vm/pkey-x86.h | 5 ++++ tools/testing/selftests/vm/protection_keys.c | 34 ++++++++++++++++--------- 3 files changed, 56 insertions(+), 12 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +29 -0
@@ -80,6 +80,35 @@ static inline void sigsafe_printf(const char *format, ...)#error Architecture not supported#endif /* arch */ +static inline pkey_reg_t clear_pkey_flags (int pkey,pkey_reg_t flags) +{ +u32 shift =pkey_bit_position (pkey); + +return ~(flags << shift); +} + +/* +* Takes pkey flags and puts them at the right bit position for the given key so +* that the result can be ORed into the register. +*/ +static inline pkey_reg_t left_shift_bits (int pkey,pkey_reg_t bits) +{ +u32 shift =pkey_bit_position (pkey); + +return (bits << shift); +} + +/* +* Takes pkey register values and puts the flags for the given pkey at the least +* significant bits of the returned value. +*/ +static inline pkey_reg_t right_shift_bits (int pkey,pkey_reg_t bits) +{ +u32 shift =pkey_bit_position (pkey); + +return (bits >> shift); +} +extern pkey_reg_t shadow_pkey_reg;static inline pkey_reg_t _read_pkey_reg (int line)
tools/testing/selftests/vm/pkey-x86.h +5 -0
@@ -49,6 +49,11 @@#define pkey_reg_t u32#define PKEY_REG_FMT "%016x" +static inline u32 pkey_bit_position (int pkey) +{ +return pkey *PKEY_BITS_PER_PKEY ; +} +static inline void __page_o_noops (void ) {/* 8-bytes of instruction * 512 bytes = 1 page */
tools/testing/selftests/vm/protection_keys.c +22 -12
@@ -342,7 +342,7 @@ static u32 hw_pkey_get(int pkey, unsigned long flags) __func__, pkey, flags, 0, 0);dprintf2 ("%s() raw pkey_reg: " PKEY_REG_FMT "\n" , __func__, pkey_reg); - shifted_pkey_reg = (pkey_reg >> (pkey *PKEY_BITS_PER_PKEY )); + shifted_pkey_reg =right_shift_bits (pkey, pkey_reg);dprintf2 ("%s() shifted_pkey_reg: " PKEY_REG_FMT "\n" , __func__, shifted_pkey_reg); masked_pkey_reg = shifted_pkey_reg & mask;
@@ -366,9 +366,9 @@ static int hw_pkey_set(int pkey, unsigned long rights, unsigned long flags)/* copy old pkey_reg */ new_pkey_reg = old_pkey_reg;/* mask out bits from pkey in old value: */ - new_pkey_reg &= ~(mask << (pkey *PKEY_BITS_PER_PKEY )); + new_pkey_reg &=clear_pkey_flags (pkey, mask);/* OR in new bits for pkey: */ - new_pkey_reg |= (rights << (pkey *PKEY_BITS_PER_PKEY )); + new_pkey_reg |=left_shift_bits (pkey, rights);__write_pkey_reg (new_pkey_reg);
@@ -402,7 +402,7 @@ void pkey_disable_set(int pkey, int flags) ret =hw_pkey_set (pkey, pkey_rights, syscall_flags);assert (!ret);/*pkey_reg and flags have the same format */ - shadow_pkey_reg |= flags << (pkey *2 ); + shadow_pkey_reg |=left_shift_bits (pkey, flags);dprintf1 ("%s(%d) shadow: 0x" PKEY_REG_FMT "\n" , __func__, pkey, shadow_pkey_reg);
@@ -436,7 +436,7 @@ void pkey_disable_clear(int pkey, int flags) pkey_rights |= flags; ret =hw_pkey_set (pkey, pkey_rights,0 ); - shadow_pkey_reg &= ~(flags << (pkey *2 )); + shadow_pkey_reg &=clear_pkey_flags (pkey, flags);pkey_assert (ret >=0 ); pkey_rights =hw_pkey_get (pkey, syscall_flags);
@@ -494,6 +494,21 @@ int sys_pkey_alloc(unsigned long flags, unsigned long init_val)return ret; } +void pkey_setup_shadow (void ) +{ + shadow_pkey_reg =__read_pkey_reg (); +} + +void pkey_reset_shadow (u32 key) +{ + shadow_pkey_reg &=clear_pkey_flags (key,0x3 ); +} + +void pkey_set_shadow (u32 key,u64 init_val) +{ + shadow_pkey_reg |=left_shift_bits (key, init_val); +} +int alloc_pkey (void ) {int ret;
@@ -512,7 +527,7 @@ int alloc_pkey(void) shadow_pkey_reg);if (ret) {/* clear both the bits: */ - shadow_pkey_reg &= ~(0x3 << (ret *2 )); +pkey_reset_shadow (ret); dprintf4("%s()::%d, ret: %d pkey_reg: 0x"PKEY_REG_FMT " shadow: 0x"PKEY_REG_FMT"\n", __func__,
@@ -522,7 +537,7 @@ int alloc_pkey(void)* move the new state in from init_val * (remember, we cheated and init_val == pkey_reg format) */ - shadow_pkey_reg |= (init_val << (ret *2 )); +pkey_set_shadow (ret, init_val); } dprintf4("%s()::%d, ret: %d pkey_reg: 0x"PKEY_REG_FMT " shadow: 0x"PKEY_REG_FMT"\n",
@@ -1403,11 +1418,6 @@ void run_tests_once(void) iteration_nr++; } -void pkey_setup_shadow (void ) -{ - shadow_pkey_reg =__read_pkey_reg (); -} -int main (void ) {int nr_iterations =22 ;
Signature · 1 line
1.7.1
PATCH v13 8/24 selftests/vm: fix the wrong assert in pkey_disable_set()
In reply to Ram PaiIf the flag is 0, no bits will be set. Hence we cant expect the resulting bitmap to have a higher value than what it was earlier.
--- tools/testing/selftests/vm/protection_keys.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
tools/testing/selftests/vm/protection_keys.c +1 -1
@@ -415,7 +415,7 @@ void pkey_disable_set(int pkey, int flags)dprintf1 ("%s(%d) pkey_reg: 0x" PKEY_REG_FMT "\n" , __func__, pkey,read_pkey_reg ());if (flags) -pkey_assert (read_pkey_reg () > orig_pkey_reg); +pkey_assert (read_pkey_reg () >= orig_pkey_reg);dprintf1 ("END<---%s(%d, 0x%x)\n" , __func__, pkey, flags); }
Signature · 1 line
1.7.1
Re: [PATCH v13 08/24] selftests/vm: fix the wrong assert in pkey_disable_set()
Quoted 3 lines · Ram Pai
If the flag is 0, no bits will be set. Hence we cant expect the resulting bitmap to have a higher value than what it was earlier
...
tools/testing/selftests/vm/protection_keys.c void pkey_disable_set(int pkey, int flags) · line 421
Quoted 6 lines
tools/testing/selftests/vm/protection_keys.c +1
-1if (flags) -pkey_assert (read_pkey_reg () > orig_pkey_reg); +pkey_assert (read_pkey_reg () >= orig_pkey_reg);dprintf1 ("END<---%s(%d, 0x%x)\n" , __func__, pkey, flags); }
This is the kind of thing where I'd love to hear the motivation and background. This "disable a key that was already disabled" operation obviously doesn't happen today. What motivated you to change it now?
Re: [PATCH v13 08/24] selftests/vm: fix the wrong assert in pkey_disable_set()
Quoted 5 lines · Dave Hansen
Quoted 3 lines · Ram Pai
If the flag is 0, no bits will be set. Hence we cant expect the resulting bitmap to have a higher value than what it was earlier
...
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c void pkey_disable_set(int pkey, int flags) · line 421
Quoted 6 lines
tools/testing/selftests/vm/protection_keys.c +1
-1if (flags) -pkey_assert (read_pkey_reg () > orig_pkey_reg); +pkey_assert (read_pkey_reg () >= orig_pkey_reg);dprintf1 ("END<---%s(%d, 0x%x)\n" , __func__, pkey, flags); }This is the kind of thing where I'd love to hear the motivation and background. This "disable a key that was already disabled" operation obviously doesn't happen today. What motivated you to change it now?
On powerpc, hardware supports READ_DISABLE and WRITE_DISABLE. ACCESS_DISABLE is basically READ_DISABLE|WRITE_DISABLE on powerpc.
If access disable is called on a key followed by a write disable, the second operation becomes a nop. In such cases,
read_pkey_reg() == orig_pkey_reg
Hence the code above is modified to
pkey_assert(read_pkey_reg() >= orig_pkey_reg);
Signature · 1 line
Ram Pai
Re: [PATCH v13 08/24] selftests/vm: fix the wrong assert in pkey_disable_set()
Quoted 18 lines · Ram Pai
Quoted 5 lines · Dave Hansen
Quoted 3 lines · Ram Pai
If the flag is 0, no bits will be set. Hence we cant expect the resulting bitmap to have a higher value than what it was earlier
...
Quoted 9 lines
tools/testing/selftests/vm/protection_keys.c void pkey_disable_set(int pkey, int flags) · line 421
Quoted 6 lines
tools/testing/selftests/vm/protection_keys.c +1
-1if (flags) -pkey_assert (read_pkey_reg () > orig_pkey_reg); +pkey_assert (read_pkey_reg () >= orig_pkey_reg);dprintf1 ("END<---%s(%d, 0x%x)\n" , __func__, pkey, flags); }This is the kind of thing where I'd love to hear the motivation and background. This "disable a key that was already disabled" operation obviously doesn't happen today. What motivated you to change it now?
On powerpc, hardware supports
READ_DISABLEandWRITE_DISABLE.ACCESS_DISABLEis basically READ_DISABLE|WRITE_DISABLE on powerpc.
Quoted 6 lines
If access disable is called on a key followed by a write disable, the second operation becomes a nop. In such cases,
read_pkey_reg() == orig_pkey_regHence the code above is modified to
pkey_assert(read_pkey_reg() >= orig_pkey_reg);
Makes sense. Do we have a comment for that now?
PATCH v13 9/24 selftests/vm: fixed bugs in pkey_disable_clear()
In reply to Ram Paiinstead of clearing the bits, pkey_disable_clear() was setting the bits. Fixed it.
Also fixed a wrong assertion in that function. When bits are cleared, the resulting bit value will be less than the original.
This hasn't been a problem so far because this code isn't currently used.
--- tools/testing/selftests/vm/protection_keys.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
tools/testing/selftests/vm/protection_keys.c +2 -2
@@ -433,7 +433,7 @@ void pkey_disable_clear(int pkey, int flags) pkey, pkey, pkey_rights);pkey_assert (pkey_rights >=0 ); - pkey_rights |= flags; + pkey_rights &= ~flags; ret =hw_pkey_set (pkey, pkey_rights,0 ); shadow_pkey_reg &=clear_pkey_flags (pkey, flags);
@@ -446,7 +446,7 @@ void pkey_disable_clear(int pkey, int flags)dprintf1 ("%s(%d) pkey_reg: 0x" PKEY_REG_FMT "\n" , __func__, pkey,read_pkey_reg ());if (flags) -assert (read_pkey_reg () > orig_pkey_reg); +assert (read_pkey_reg () < orig_pkey_reg); }void pkey_write_allow (int pkey)
Signature · 1 line
1.7.1
PATCH v13 10/24 selftests/vm: clear the bits in shadow reg when a pkey is freed.
In reply to Ram PaiWhen a key is freed, the key is no more effective. Clear the bits corresponding to the pkey in the shadow register. Otherwise it will carry some spurious bits which can trigger false-positive asserts.
--- tools/testing/selftests/vm/protection_keys.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/protection_keys.c +3 -0
@@ -556,6 +556,9 @@ int alloc_pkey(void)int sys_pkey_free (unsigned long pkey) {int ret =syscall (SYS_pkey_free, pkey); + +if (!ret) + shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }
Signature · 1 line
1.7.1
Re: [PATCH v13 10/24] selftests/vm: clear the bits in shadow reg when a pkey is freed.
Quoted 4 lines · Ram Pai
When a key is freed, the key is no more effective. Clear the bits corresponding to the pkey in the shadow register. Otherwise it will carry some spurious bits which can trigger false-positive asserts.
...--- a/tools/testing/selftests/vm/protection_keys.c
tools/testing/selftests/vm/protection_keys.c int alloc_pkey(void) · line 564
Quoted 11 lines
+++
b/tools/testing/selftests/vm/protection_keys.ctools/testing/selftests/vm/protection_keys.c +3
-0@@ -556,6 +556,9 @@ int alloc_pkey(void)int sys_pkey_free (unsigned long pkey) {int ret =syscall (SYS_pkey_free, pkey); + +if (!ret) + shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }
This would be great code for an actual application. But, I'm not immediately convinced we want sane, kind behavior in our selftest. x86 doesn't clear the hardware register at pkey_free, so wouldn't this cause the shadow and the hardware register to diverge?
Re: [PATCH v13 10/24] selftests/vm: clear the bits in shadow reg when a pkey is freed.
Quoted 6 lines · Dave Hansen
Quoted 4 lines · Ram Pai
When a key is freed, the key is no more effective. Clear the bits corresponding to the pkey in the shadow register. Otherwise it will carry some spurious bits which can trigger false-positive asserts.
...---
a/tools/testing/selftests/vm/protection_keys.c
Quoted 16 lines
tools/testing/selftests/vm/protection_keys.c int alloc_pkey(void) · line 564
Quoted 11 lines
+++
b/tools/testing/selftests/vm/protection_keys.ctools/testing/selftests/vm/protection_keys.c +3
-0@@ -556,6 +556,9 @@ int alloc_pkey(void)int sys_pkey_free (unsigned long pkey) {int ret =syscall (SYS_pkey_free, pkey); + +if (!ret) + shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }This would be great code for an actual application. But, I'm not immediately convinced we want sane, kind behavior in our selftest. x86 doesn't clear the hardware register at
pkey_free, so wouldn't this cause the shadow and the hardware register to diverge?
Have deleted the code in the newer version.
RP
PATCH v13 11/24 selftests/vm: fix alloc_random_pkey() to make it really random
In reply to Ram Paialloc_random_pkey() was allocating the same pkey every time. Not all pkeys were geting tested. fixed it.
--- tools/testing/selftests/vm/protection_keys.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
tools/testing/selftests/vm/protection_keys.c +5 -1
@@ -24,6 +24,7 @@#define _GNU_SOURCE#include <errno.h> #include <linux/futex.h> +#include <time.h> #include <sys/time.h> #include <sys/syscall.h> #include <string.h>
@@ -576,13 +577,15 @@ int alloc_random_pkey(void)int alloced_pkeys[NR_PKEYS ];int nr_alloced =0 ;int random_index; +memset (alloced_pkeys,0 ,sizeof (alloced_pkeys)); +srand ((unsigned int )time (NULL ));/* allocate every possible key and make a note of which ones we got */ max_nr_pkey_allocs =NR_PKEYS ; - max_nr_pkey_allocs =1 ;for (i =0 ; i < max_nr_pkey_allocs; i++) {int new_pkey =alloc_pkey (); +if (new_pkey <0 )break ; alloced_pkeys[nr_alloced++] = new_pkey;
@@ -598,6 +601,7 @@ int alloc_random_pkey(void)/* go through the allocated ones that we did not want and free them */ for (i =0 ; i < nr_alloced; i++) {int free_ret; +if (!alloced_pkeys[i])continue ; free_ret =sys_pkey_free (alloced_pkeys[i]);
Signature · 1 line
1.7.1
PATCH v13 12/24 selftests/vm: introduce two arch independent abstraction
In reply to Ram Paiopen_hugepage_file() <- opens the huge page file get_start_key() <-- provides the first non-reserved key.
--- tools/testing/selftests/vm/pkey-helpers.h | 10 ++++++++++ tools/testing/selftests/vm/pkey-x86.h | 1 + tools/testing/selftests/vm/protection_keys.c | 6 +++--- 3 files changed, 14 insertions(+), 3 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +10 -0
@@ -179,4 +179,14 @@ static inline void __pkey_write_allow(int pkey, int do_allow_write) #define __stringify_1(x...) #x #define __stringify(x...) __stringify_1(x) +static inline int open_hugepage_file (int flag) +{ +return open (HUGEPAGE_FILE , flag); +} + +static inline int get_start_key (void ) +{ +return 1 ; +} +#endif /* _PKEYS_HELPER_H */
tools/testing/selftests/vm/pkey-x86.h +1 -0
@@ -48,6 +48,7 @@#define MB (1<<20)#define pkey_reg_t u32#define PKEY_REG_FMT "%016x" +#define HUGEPAGE_FILE "/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages"static inline u32 pkey_bit_position (int pkey) {
tools/testing/selftests/vm/protection_keys.c +3 -3
@@ -791,7 +791,7 @@ void setup_hugetlbfs(void)* Now go make sure that we got the pages and that they * are 2M pages. Someone might have made 1G the default. */ - fd =open ("/sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages" ,O_RDONLY ); + fd =open_hugepage_file (O_RDONLY );if (fd <0 ) {perror ("opening sysfs 2M hugetlb config" );return ;
@@ -1078,10 +1078,10 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)void test_pkey_syscalls_on_non_allocated_pkey (int *ptr,u16 pkey) {int err; -int i; +int i =get_start_key ();/* Note: 0 is the default pkey, so don't mess with it */ -for (i =1 ; i <NR_PKEYS ; i++) { +for (; i <NR_PKEYS ; i++) {if (pkey == i)continue ;
Signature · 1 line
1.7.1
PATCH v13 13/24 selftests/vm: pkey register should match shadow pkey
In reply to Ram Paiexpected_pkey_fault() is comparing the contents of pkey register with 0. This may not be true all the time. There could be bits set by default by the architecture which can never be changed. Hence compare the value against shadow pkey register, which is supposed to track the bits accurately all throughout
--- tools/testing/selftests/vm/protection_keys.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
tools/testing/selftests/vm/protection_keys.c +2 -2
@@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey)pkey_assert (last_si_pkey == pkey);/* -* The signal handler shold have cleared out PKEY register to let the +* The signal handler should have cleared out pkey-register to let the * test program continue. We now have to restore it. */ -if (__read_pkey_reg () !=0 ) +if (__read_pkey_reg () != shadow_pkey_reg)pkey_assert (0 );__write_pkey_reg (shadow_pkey_reg);
Signature · 1 line
1.7.1
Re: [PATCH v13 13/24] selftests/vm: pkey register should match shadow pkey
Quoted 4 lines · Ram Pai
+++
b/tools/testing/selftests/vm/protection_keys.ctools/testing/selftests/vm/protection_keys.c +0
-0@@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey)pkey_assert (last_si_pkey == pkey);
tools/testing/selftests/vm/protection_keys.c void expected_pkey_fault(int pkey) · line 925
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +2
-2/* -* The signal handler shold have cleared out PKEY register to let the +* The signal handler should have cleared out pkey-register to let the * test program continue. We now have to restore it. */ -if (__read_pkey_reg () !=0 ) +if (__read_pkey_reg () != shadow_pkey_reg)pkey_assert (0 );__write_pkey_reg (shadow_pkey_reg);
I think this is wrong on x86.
When we leave the signal handler, we zero out PKRU so that the faulting instruction can continue, that's why we have the check against zero. I'm actually kinda surprised this works.
Logically, this patch does:
if (hardware != shadow) error(); hardware = shadow;
That does not look right to me. What we want is:
if (hardware != signal_return_pkey_reg) error(); hardware = shadow;
Re: [PATCH v13 13/24] selftests/vm: pkey register should match shadow pkey
Quoted 14 lines · Dave Hansen
Quoted 13 lines · Ram Pai
+++
b/tools/testing/selftests/vm/protection_keys.ctools/testing/selftests/vm/protection_keys.c +2
-2@@ -916,10 +916,10 @@ void expected_pkey_fault(int pkey)pkey_assert (last_si_pkey == pkey);/* -* The signal handler shold have cleared out PKEY register to let the +* The signal handler should have cleared out pkey-register to let the * test program continue. We now have to restore it. */ -if (__read_pkey_reg () !=0 ) +if (__read_pkey_reg () != shadow_pkey_reg)pkey_assert (0 );
Quoted 7 lines
tools/testing/selftests/vm/protection_keys.c void expected_pkey_fault(int pkey) · line 925
Quoted 1 line
tools/testing/selftests/vm/protection_keys.c +0
-0__write_pkey_reg (shadow_pkey_reg);I think this is wrong on x86.
When we leave the signal handler, we zero out PKRU so that the faulting instruction can continue, that's why we have the check against zero. I'm actually kinda surprised this works.
The code is modified to zero out only the violated key in the signal handler. Hence it works. Have verified it to work on x86 aswell.
RP
PATCH v13 14/24 selftests/vm: generic cleanup
In reply to Ram Paicleanup the code to satisfy coding styles.
--- tools/testing/selftests/vm/protection_keys.c | 64 +++++++++++++++++-------- 1 files changed, 43 insertions(+), 21 deletions(-)
tools/testing/selftests/vm/protection_keys.c +43 -21
@@ -4,7 +4,7 @@* * There are examples in here of: * * how to set protection keys on memory -* * how to set/clear bits in pkey registers (the rights register) +* * how to set/clear bits in Protection Key registers (the rights register) * * how to handle SEGV_PKUERR signals and extract pkey-relevant * information from the siginfo *
@@ -13,13 +13,18 @@* prefault pages in at malloc, or not * protect MPX bounds tables with protection keys? * make sure VMA splitting/merging is working correctly -* OOMs can destroy mm->mmap (see exit_mmap()), so make sure it is immune to pkeys -* look for pkey "leaks" where it is still set on a VMA but "freed" back to the kernel -* do a plain mprotect() to a mprotect_pkey() area and make sure the pkey sticks +* OOMs can destroy mm->mmap (see exit_mmap()), +* so make sure it is immune to pkeys +* look for pkey "leaks" where it is still set on a VMA +* but "freed" back to the kernel +* do a plain mprotect() to a mprotect_pkey() area and make +* sure the pkey sticks * * Compile like this: -* gcc -o protection_keys -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm -* gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 -pthread -Wall protection_keys.c -lrt -ldl -lm +* gcc -o protection_keys -O2 -g -std=gnu99 +* -pthread -Wall protection_keys.c -lrt -ldl -lm +* gcc -m32 -o protection_keys_32 -O2 -g -std=gnu99 +* -pthread -Wall protection_keys.c -lrt -ldl -lm */ #define _GNU_SOURCE#include <errno.h>
@@ -263,10 +268,12 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) __read_pkey_reg());dprintf1 ("pkey from siginfo: %jx\n" , siginfo_pkey); *(u64 *)pkey_reg_ptr =0x00000000 ; -dprintf1 ("WARNING: set PRKU=0 to allow faulting instruction to continue\n" ); +dprintf1 ("WARNING: set PKEY_REG=0 to allow faulting instruction " +"to continue\n" ); pkey_faults++;dprintf1 ("<<<<==================================================\n" ); dprint_in_signal =0 ; +return ; }int wait_all_children (void )
@@ -384,7 +391,7 @@ void pkey_disable_set(int pkey, int flags) {unsigned long syscall_flags =0 ;int ret; -int pkey_rights; +u32 pkey_rights;pkey_reg_t orig_pkey_reg =read_pkey_reg (); dprintf1("START->%s(%d, 0x%x)\n", __func__,
@@ -487,9 +494,10 @@ int sys_mprotect_pkey(void *ptr, size_t size, unsigned long orig_prot,return sret; } -int sys_pkey_alloc (unsigned long flags,unsigned long init_val) +int sys_pkey_alloc (unsigned long flags,u64 init_val) {int ret =syscall (SYS_pkey_alloc, flags, init_val); +dprintf1 ("%s(flags=%lx, init_val=%lx) syscall ret: %d errno: %d\n" , __func__, flags, init_val, ret, errno);return ret;
@@ -513,7 +521,7 @@ void pkey_set_shadow(u32 key, u64 init_val)int alloc_pkey (void ) {int ret; -unsigned long init_val =0x0 ; +u64 init_val =0x0 ;dprintf1 ("%s()::%d, pkey_reg: " PKEY_REG_FMT " shadow: " PKEY_REG_FMT "\n" , __func__, __LINE__,__read_pkey_reg (), shadow_pkey_reg);
@@ -672,7 +680,9 @@ void record_pkey_malloc(void *ptr, long size, int prot)/* every record is full */ size_t old_nr_records = nr_pkey_malloc_records;size_t new_nr_records = (nr_pkey_malloc_records *2 +1 ); -size_t new_size = new_nr_records *sizeof (struct pkey_malloc_record ); +size_t new_size = new_nr_records * +sizeof (struct pkey_malloc_record ); +dprintf2 ("new_nr_records: %zd\n" , new_nr_records);dprintf2 ("new_size: %zd\n" , new_size); pkey_malloc_records =realloc (pkey_malloc_records, new_size);
@@ -698,9 +708,11 @@ void free_pkey_malloc(void *ptr) {long i;int ret; +dprintf3 ("%s(%p)\n" , __func__, ptr);for (i =0 ; i < nr_pkey_malloc_records; i++) {struct pkey_malloc_record *rec = &pkey_malloc_records[i]; +dprintf4 ("looking for ptr %p at record[%ld/%p]: {%p, %ld}\n" , ptr, i, rec, rec->ptr, rec->size); if ((ptr < rec->ptr) ||
@@ -781,11 +793,13 @@ void setup_hugetlbfs(void)char buf[] ="123" ;if (geteuid () !=0 ) { -fprintf (stderr,"WARNING: not run as root, can not do hugetlb test\n" ); +fprintf (stderr, +"WARNING: not run as root, can not do hugetlb test\n" );return ; } -cat_into_file (__stringify (GET_NR_HUGE_PAGES ),"/proc/sys/vm/nr_hugepages" ); +cat_into_file (__stringify (GET_NR_HUGE_PAGES ), +"/proc/sys/vm/nr_hugepages" );/* * Now go make sure that we got the pages and that they
@@ -806,7 +820,8 @@ void setup_hugetlbfs(void) }if (atoi (buf) !=GET_NR_HUGE_PAGES ) { -fprintf (stderr,"could not confirm 2M pages, got: '%s' expected %d\n" , +fprintf (stderr,"could not confirm 2M pages, got:" +" '%s' expected %d\n" , buf,GET_NR_HUGE_PAGES );return ; }
@@ -948,6 +963,7 @@ void __save_test_fd(int fd)int get_test_read_fd (void ) {int test_fd =open ("/etc/passwd" ,O_RDONLY ); +__save_test_fd (test_fd);return test_fd; }
@@ -989,7 +1005,8 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey) {int ptr_contents; -dprintf1 ("disabling access to PKEY[%02d], doing read @ %p\n" , pkey, ptr); +dprintf1 ("disabling access to PKEY[%02d], doing read @ %p\n" , + pkey, ptr);read_pkey_reg ();pkey_access_deny (pkey); ptr_contents =read_ptr (ptr);
@@ -1111,13 +1128,14 @@ void test_pkey_syscalls_bad_args(int *ptr, u16 pkey)/* Assumes that all pkeys other than 'pkey' are unallocated */ void test_pkey_alloc_exhaust (int *ptr,u16 pkey) { -int err; +int err =0 ;int allocated_pkeys[NR_PKEYS ] = {0 };int nr_allocated_pkeys =0 ;int i;for (i =0 ; i <NR_PKEYS *2 ; i++) {int new_pkey; +dprintf1 ("%s() alloc loop: %d\n" , __func__, i); new_pkey =alloc_pkey (); dprintf4("%s()::%d, err: %d pkey_reg: 0x"PKEY_REG_FMT
@@ -1125,9 +1143,11 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey) __func__, __LINE__, err,__read_pkey_reg (), shadow_pkey_reg);read_pkey_reg ();/* for shadow checking */ -dprintf2 ("%s() errno: %d ENOSPC: %d\n" , __func__, errno,ENOSPC ); +dprintf2 ("%s() errno: %d ENOSPC: %d\n" , + __func__, errno,ENOSPC );if ((new_pkey ==-1 ) && (errno ==ENOSPC )) { -dprintf2 ("%s() failed to allocate pkey after %d tries\n" , +dprintf2 ("%s() failed to allocate pkey " +"after %d tries\n" , __func__, nr_allocated_pkeys);break ; }
@@ -1419,7 +1439,8 @@ void run_tests_once(void)tracing_off ();close_test_fds (); -printf ("test %2d PASSED (iteration %d)\n" , test_nr, iteration_nr); +printf ("test %2d PASSED (iteration %d)\n" , + test_nr, iteration_nr);dprintf1 ("======================\n\n" ); } iteration_nr++;
@@ -1431,7 +1452,7 @@ int main(void)setup_handlers (); -printf ("has pku: %d\n" ,cpu_has_pku ()); +printf ("has pkey: %d\n" ,cpu_has_pku ());if (!cpu_has_pku ()) {int size =PAGE_SIZE ;
@@ -1439,7 +1460,8 @@ int main(void)printf ("running PKEY tests for unsupported CPU/OS\n" ); - ptr =mmap (NULL , size,PROT_NONE ,MAP_ANONYMOUS |MAP_PRIVATE ,-1 ,0 ); + ptr =mmap (NULL , size,PROT_NONE , +MAP_ANONYMOUS |MAP_PRIVATE ,-1 ,0 );assert (ptr != (void *)-1 );test_mprotect_pkey_on_unsupported_cpu (ptr,1 );exit (0 );
Signature · 1 line
1.7.1
Re: [PATCH v13 14/24] selftests/vm: generic cleanup
Quoted 1 line · Ram Pai
cleanup the code to satisfy coding styles.
A lot of this makes the code look worse and more unreadable than before.
I think someone just ran it through lindent or something.
I also took a few CodingStyle liberties in here because it's _not_ main kernel code. I think the occasional 85-column line is probably OK in here.
PATCH v13 15/24 selftests/vm: powerpc implementation for generic abstraction
In reply to Ram PaiIntroduce powerpc implementation for the various
abstractions.
--- tools/testing/selftests/vm/pkey-helpers.h | 16 ++++- tools/testing/selftests/vm/pkey-powerpc.h | 91 ++++++++++++++++++++++++++ tools/testing/selftests/vm/pkey-x86.h | 15 ++++ tools/testing/selftests/vm/protection_keys.c | 62 ++++++++++-------- 4 files changed, 156 insertions(+), 28 deletions(-) create mode 100644 tools/testing/selftests/vm/pkey-powerpc.h
tools/testing/selftests/vm/pkey-helpers.h +15 -1
@@ -74,8 +74,13 @@ static inline void sigsafe_printf(const char *format, ...) } \ }while (0 ) +__attribute__((noinline)) int read_ptr(int *ptr); +void expected_pkey_fault (int pkey); +#if defined(__i386__) || defined(__x86_64__)/* arch */ #include "pkey-x86.h" +#elif defined(__powerpc64__)/* arch */ +#include "pkey-powerpc.h" #else /* arch */ #error Architecture not supported#endif /* arch */
@@ -186,7 +191,16 @@ static inline int open_hugepage_file(int flag)static inline int get_start_key (void ) { -return 1 ; +return 0 ; +} + +static inline u32 *siginfo_get_pkey_ptr (siginfo_t *si) +{ +#ifdef si_pkey +return &si->si_pkey; +#else +return (u32 *)(((u8 *)si) + si_pkey_offset); +#endif }#endif /* _PKEYS_HELPER_H */
tools/testing/selftests/vm/pkey-powerpc.h new file +91 -0
91 lines not on this page · raw message
tools/testing/selftests/vm/pkey-x86.h +15 -0
@@ -42,6 +42,7 @@#endif #define NR_PKEYS 16 +#define NR_RESERVED_PKEYS 1#define PKEY_BITS_PER_PKEY 2#define HPAGE_SIZE (1UL<<21)#define PAGE_SIZE 4096
@@ -161,4 +162,18 @@ int pkey_reg_xstate_offset(void) return xstate_offset; } +static inline int arch_reserved_keys (void ) +{ +return NR_RESERVED_PKEYS ; +} + +void expect_fault_on_read_execonly_key (void *p1,u16 pkey) +{ +int ptr_contents; + + ptr_contents =read_ptr (p1); +dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); +expected_pkey_fault (pkey); +} +#endif /* _PKEYS_X86_H */
tools/testing/selftests/vm/protection_keys.c +35 -27
124 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [PATCH v13 15/24] selftests/vm: powerpc implementation for generic abstraction
tools/testing/selftests/vm/pkey-helpers.h static inline int open_hugepage_file(int flag) · line 204
Quoted 8 lines
tools/testing/selftests/vm/pkey-helpers.h +7
-0+static inline u32 *siginfo_get_pkey_ptr (siginfo_t *si) +{ +#ifdef si_pkey +return &si->si_pkey; +#else +return (u32 *)(((u8 *)si) + si_pkey_offset); +#endif }
FWIW, this isn't ppc-specific.
Quoted 55 lines
tools/testing/selftests/vm/protection_keys.c +10
-10@@ -197,17 +197,18 @@ void dump_mem(void *dumpme, int len_bytes)int pkey_faults;int last_si_pkey =-1 ; +void pkey_access_allow (int pkey);void signal_handler (int signum,siginfo_t *si,void *vucontext) {ucontext_t *uctxt = vucontext;int trapno;unsigned long ip;char *fpregs; +#if defined(__i386__) || defined(__x86_64__)/* arch */ pkey_reg_t *pkey_reg_ptr; -u64 siginfo_pkey; +#endif /* defined(__i386__) || defined(__x86_64__) */ +u32 siginfo_pkey;u32 *si_pkey_ptr; -int pkey_reg_offset; -fpregset_t fpregset; dprint_in_signal =1 ;dprintf1 (">>>>===============SIGSEGV============================\n" );@@ -217,12 +218,14 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext) trapno = uctxt->uc_mcontext.gregs[REG_TRAPNO ]; ip = uctxt->uc_mcontext.gregs[REG_IP_IDX ]; - fpregset = uctxt->uc_mcontext.fpregs; - fpregs = (void *)fpregset; + fpregs = (char *) uctxt->uc_mcontext.fpregs;dprintf2 ("%s() trapno: %d ip: 0x%016lx info->si_code: %s/%d\n" , __func__, trapno, ip,si_code_str (si->si_code), si->si_code); + +#if defined(__i386__) || defined(__x86_64__)/* arch */ +#ifdef __i386__/* * 32-bit has some extra padding so that userspace can tell whether @@ -230,20 +233,28 @@ void signal_handler(int signum, siginfo_t *si, void *vucontext)* state. We just assume that it is here. */ fpregs +=0x70 ; -#endif - pkey_reg_offset =pkey_reg_xstate_offset (); - pkey_reg_ptr = (void *)(&fpregs[pkey_reg_offset]); +#endif /* __i386__ */ -dprintf1 ("siginfo: %p\n" , si); -dprintf1 (" fpregs: %p\n" , fpregs); + pkey_reg_ptr = (void *)(&fpregs[pkey_reg_xstate_offset ()]);
tools/testing/selftests/vm/protection_keys.c void signal_handler(int signum, siginfo_t *si, void *vucontext) · line 247
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +2
-1/* -* If we got a PKEY fault, we *HAVE* to have at least one bit set in +* If we got a key fault, we *HAVE* to have at least one bit set in * here. */ dprintf1 ("pkey_reg_xstate_offset: %d\n" ,pkey_reg_xstate_offset ());if (DEBUG_LEVEL >4 )dump_mem (pkey_reg_ptr -128 ,256 );pkey_assert (*pkey_reg_ptr); +#endif /* defined(__i386__) || defined(__x86_64__) */
The series up to this point has been looking pretty nice and broken out and easy to read. It goes off the rails a bit here. Adding #ifdefs and..
Quoted 13 lines
tools/testing/selftests/vm/protection_keys.c +9
-0+dprintf1 ("siginfo: %p\n" , si); +dprintf1 (" fpregs: %p\n" , fpregs); + + si_pkey_ptr =siginfo_get_pkey_ptr (si); +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) ||
tools/testing/selftests/vm/protection_keys.c void signal_handler(int signum, siginfo_t *si, void *vucontext) · line 260
Quoted 9 lines
tools/testing/selftests/vm/protection_keys.c +0
-6@@ -252,22 +263,21 @@ 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;
Moving random code around with no explanation.
Quoted 22 lines
tools/testing/selftests/vm/protection_keys.c +9
-3-dprintf1 ("signal pkey_reg from xsave: " PKEY_REG_FMT "\n" , *pkey_reg_ptr);/* * need __read_pkey_reg() version so we do not do shadow_pkey_reg * checking */ dprintf1 ("signal pkey_reg from pkey_reg: " PKEY_REG_FMT "\n" ,__read_pkey_reg ()); -dprintf1 ("pkey from siginfo: %jx\n" , siginfo_pkey); - *(u64 *)pkey_reg_ptr =0x00000000 ; +#if defined(__i386__) || defined(__x86_64__)/* arch */ +dprintf1 ("signal pkey_reg from xsave: " PKEY_REG_FMT "\n" , *pkey_reg_ptr); + *(u64 *)pkey_reg_ptr &=clear_pkey_flags (siginfo_pkey, +PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE ); +#elif __powerpc64__ +pkey_access_allow (siginfo_pkey); +#endif + shadow_pkey_reg &=clear_pkey_flags (siginfo_pkey, +PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );dprintf1 ("WARNING: set PKEY_REG=0 to allow faulting instruction " "to continue\n" ); pkey_faults++;
tools/testing/selftests/vm/protection_keys.c void test_executing_on_unreadable_memory(int *ptr, u16 pkey) · line 1346
Quoted 9 lines
tools/testing/selftests/vm/protection_keys.c +2
-3@@ -1331,9 +1341,8 @@ 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_pkey_fault ("executing on PROT_EXEC memory" ); - ptr_contents =read_ptr (p1); -dprintf2 ("ptr (%p) contents@%d: %x\n" , p1, __LINE__, ptr_contents); -expected_pkey_fault (pkey); + +expect_fault_on_read_execonly_key (p1, pkey); }
While none of this is a deal-breaker (as I said, I feel like the selftests/ rules are a bit more lax) this does kinda break the illusion of a nice, broken out series.
Could you address this a bit in the changelog at least, please?
PATCH v13 16/24 selftests/vm: clear the bits in shadow reg when a pkey is freed.
In reply to Ram PaiWhen a key is freed, the key is no more effective. Clear the bits corresponding to the pkey in the shadow register. Otherwise it will carry some spurious bits which can trigger false-positive asserts.
--- tools/testing/selftests/vm/protection_keys.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
tools/testing/selftests/vm/protection_keys.c +2 -1
@@ -577,7 +577,8 @@ int sys_pkey_free(unsigned long pkey)int ret =syscall (SYS_pkey_free, pkey);if (!ret) - shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS ); + shadow_pkey_reg &=clear_pkey_flags (pkey, +PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }
Signature · 1 line
1.7.1
Re: [PATCH v13 16/24] selftests/vm: clear the bits in shadow reg when a pkey is freed.
tools/testing/selftests/vm/protection_keys.c int sys_pkey_free(unsigned long pkey) · line 584
Quoted 12 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +2
-1@@ -577,7 +577,8 @@ int sys_pkey_free(unsigned long pkey)int ret =syscall (SYS_pkey_free, pkey);if (!ret) - shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS ); + shadow_pkey_reg &=clear_pkey_flags (pkey, +PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }
Why did you introduce this code earlier and then modify it now?
BTW, my original aversion to this code still stands.
Re: [PATCH v13 16/24] selftests/vm: clear the bits in shadow reg when a pkey is freed.
Quoted 16 lines · Dave Hansen
tools/testing/selftests/vm/protection_keys.c int sys_pkey_free(unsigned long pkey) · line 584
Quoted 12 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +2
-1@@ -577,7 +577,8 @@ int sys_pkey_free(unsigned long pkey)int ret =syscall (SYS_pkey_free, pkey);if (!ret) - shadow_pkey_reg &=clear_pkey_flags (pkey,PKEY_DISABLE_ACCESS ); + shadow_pkey_reg &=clear_pkey_flags (pkey, +PKEY_DISABLE_ACCESS |PKEY_DISABLE_WRITE );dprintf1 ("%s(pkey=%ld) syscall ret: %d\n" , __func__, pkey, ret);return ret; }Why did you introduce this code earlier and then modify it now?
Quoted 1 line
BTW, my original aversion to this code still stands.
Have entirely got rid of this code in the new version.
Signature · 1 line
Ram Pai
PATCH v13 17/24 selftests/vm: powerpc implementation to check support for pkey
In reply to Ram Paipkey subsystem is supported if the hardware and kernel has support. We determine that by checking if allocation of a key succeeds or not.
--- tools/testing/selftests/vm/pkey-helpers.h | 2 ++ tools/testing/selftests/vm/pkey-powerpc.h | 14 ++++++++++++-- tools/testing/selftests/vm/pkey-x86.h | 8 ++++---- tools/testing/selftests/vm/protection_keys.c | 9 +++++---- 4 files changed, 23 insertions(+), 10 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +2 -0
@@ -76,6 +76,8 @@ static inline void sigsafe_printf(const char *format, ...) __attribute__((noinline))int read_ptr (int *ptr);void expected_pkey_fault (int pkey); +int sys_pkey_alloc (unsigned long flags,u64 init_val); +int sys_pkey_free (unsigned long pkey);#if defined(__i386__) || defined(__x86_64__)/* arch */ #include "pkey-x86.h"
tools/testing/selftests/vm/pkey-powerpc.h +12 -2
@@ -62,9 +62,19 @@ static inline void __write_pkey_reg(pkey_reg_t pkey_reg) pkey_reg); } -static inline int cpu_has_pku (void ) +static inline bool is_pkey_supported (void ) { -return 1 ; +/* +* No simple way to determine this. +* Lets try allocating a key and see if it succeeds. +*/ +int ret =sys_pkey_alloc (0 ,0 ); + +if (ret >0 ) { +sys_pkey_free (ret); +return true; + } +return false; }static inline int arch_reserved_keys (void )
tools/testing/selftests/vm/pkey-x86.h +4 -4
@@ -105,7 +105,7 @@ static inline void __cpuid(unsigned int *eax, unsigned int *ebx,#define X86_FEATURE_PKU (1<<3)/* Protection Keys for Userspace */ #define X86_FEATURE_OSPKE (1<<4)/* OS Protection Keys Enable */ -static inline int cpu_has_pku (void ) +static inline bool is_pkey_supported (void ) {unsigned int eax;unsigned int ebx;
@@ -118,13 +118,13 @@ static inline int cpu_has_pku(void)if (!(ecx &X86_FEATURE_PKU )) {dprintf2 ("cpu does not have PKU\n" ); -return 0 ; +return false; }if (!(ecx &X86_FEATURE_OSPKE )) {dprintf2 ("cpu does not have OSPKE\n" ); -return 0 ; +return false; } -return 1 ; +return true; }#define XSTATE_PKEY_BIT (9)
tools/testing/selftests/vm/protection_keys.c +5 -4
@@ -1393,8 +1393,8 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)int size =PAGE_SIZE ;int sret; -if (cpu_has_pku ()) { -dprintf1 ("SKIP: %s: no CPU support\n" , __func__); +if (is_pkey_supported ()) { +dprintf1 ("SKIP: %s: no CPU/kernel support\n" , __func__);return ; }
@@ -1458,12 +1458,13 @@ void run_tests_once(void)int main (void ) {int nr_iterations =22 ; +int pkey_supported =is_pkey_supported ();setup_handlers (); -printf ("has pkey: %d\n" ,cpu_has_pku ()); +printf ("has pkey: %s\n" , pkey_supported ?"Yes" :"No" ); -if (!cpu_has_pku ()) { +if (!pkey_supported) {int size =PAGE_SIZE ;int *ptr;
Signature · 1 line
1.7.1
Re: [PATCH v13 17/24] selftests/vm: powerpc implementation to check support for pkey
tools/testing/selftests/vm/protection_keys.c void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) · line 1399
Quoted 6 lines
tools/testing/selftests/vm/protection_keys.c +2
-2-if (cpu_has_pku ()) { -dprintf1 ("SKIP: %s: no CPU support\n" , __func__); +if (is_pkey_supported ()) { +dprintf1 ("SKIP: %s: no CPU/kernel support\n" , __func__);return ; }
I actually kinda wanted a specific message for when the *CPU* doesn't support the feature.
Re: [PATCH v13 17/24] selftests/vm: powerpc implementation to check support for pkey
Quoted 9 lines · Dave Hansen
tools/testing/selftests/vm/protection_keys.c void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) · line 1399
Quoted 6 lines
tools/testing/selftests/vm/protection_keys.c +2
-2-if (cpu_has_pku ()) { -dprintf1 ("SKIP: %s: no CPU support\n" , __func__); +if (is_pkey_supported ()) { +dprintf1 ("SKIP: %s: no CPU/kernel support\n" , __func__);return ; }I actually kinda wanted a specific message for when the *CPU* doesn't support the feature.
is_pkey_supported() x86 implementation has specific messages. it will print if the CPU doesn't support the feature.
RP
Signature · 1 line
Ram Pai
PATCH v13 18/24 selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
In reply to Ram PaiThe maximum number of keys that can be allocated has to take into consideration, that some keys are reserved by
the architecture for specific purpose. Hence cannot
be allocated.
Fix the assertion in test_pkey_alloc_exhaust()
--- tools/testing/selftests/vm/protection_keys.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-)
tools/testing/selftests/vm/protection_keys.c +5 -8
@@ -1175,15 +1175,12 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)pkey_assert (i <NR_PKEYS *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). +* There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys() +* are reserved. One of which is the default key(0). One can be taken +* up by an execute-only mapping. +* Ensure that we can allocate at least the remaining. */ -pkey_assert (i >=NR_PKEYS -3 ); +pkey_assert (i >= (NR_PKEYS -arch_reserved_keys ()-1 ));for (i =0 ; i < nr_allocated_pkeys; i++) { err =sys_pkey_free (allocated_pkeys[i]);
Signature · 1 line
1.7.1
Re: [PATCH v13 18/24] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
Quoted 5 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +0
-4/* -* 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.
tools/testing/selftests/vm/protection_keys.c void test_pkey_alloc_exhaust(int *ptr, u16 pkey) · line 1183
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +5
-4-* 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). +* There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys() +* are reserved. One of which is the default key(0). One can be taken +* up by an execute-only mapping. +* Ensure that we can allocate at least the remaining. */ -pkey_assert (i >=NR_PKEYS -3 ); +pkey_assert (i >= (NR_PKEYS -arch_reserved_keys ()-1 ));
We recently had a bug here. I fixed it and left myself a really nice comment so I and others wouldn't screw it up in the future.
Does this kill my nice, new comment?
Re: [PATCH v13 18/24] selftests/vm: fix an assertion in test_pkey_alloc_exhaust()
Quoted 20 lines · Dave Hansen
Quoted 5 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +0
-4/* -* 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. tools/testing/selftests/vm/protection_keys.c void test_pkey_alloc_exhaust(int *ptr, u16 pkey) · line 1183
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +5
-4-* 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). +* There are NR_PKEYS pkeys supported in hardware. arch_reserved_keys() +* are reserved. One of which is the default key(0). One can be taken +* up by an execute-only mapping. +* Ensure that we can allocate at least the remaining. */ -pkey_assert (i >=NR_PKEYS -3 ); +pkey_assert (i >= (NR_PKEYS -arch_reserved_keys ()-1 ));We recently had a bug here. I fixed it and left myself a really nice comment so I and others wouldn't screw it up in the future.
Quoted 1 line
Does this kill my nice, new comment?
part of your nice comment has been moved into the header file. The arch specific header file explains where and how the reserved keys are used.
Signature · 1 line
Ram Pai
PATCH v13 19/24 selftests/vm: associate key on a mapped page and detect access violation
In reply to Ram Paidetect access-violation on a page to which access-disabled key is associated much after the page is mapped.
--- tools/testing/selftests/vm/protection_keys.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/protection_keys.c +19 -0
@@ -1024,6 +1024,24 @@ void test_read_of_access_disabled_region(int *ptr, u16 pkey)dprintf1 ("*ptr: %d\n" , ptr_contents);expected_pkey_fault (pkey); } + +void test_read_of_access_disabled_region_with_page_already_mapped (int *ptr, +u16 pkey) +{ +int ptr_contents; + +dprintf1 ("disabling access to PKEY[%02d], doing read @ %p\n" , + pkey, ptr); + ptr_contents =read_ptr (ptr); +dprintf1 ("reading ptr before disabling the read : %d\n" , + ptr_contents); +read_pkey_reg (); +pkey_access_deny (pkey); + ptr_contents =read_ptr (ptr); +dprintf1 ("*ptr: %d\n" , ptr_contents); +expected_pkey_fault (pkey); +} +void test_write_of_write_disabled_region (int *ptr,u16 pkey) {dprintf1 ("disabling write access to PKEY[%02d], doing write\n" , pkey);
@@ -1402,6 +1420,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey)void (*pkey_tests[])(int *ptr,u16 pkey) = { test_read_of_write_disabled_region, test_read_of_access_disabled_region, + test_read_of_access_disabled_region_with_page_already_mapped, test_write_of_write_disabled_region, test_write_of_access_disabled_region, test_kernel_write_of_access_disabled_region,
Signature · 1 line
1.7.1
Re: [PATCH v13 19/24] selftests/vm: associate key on a mapped page and detect access violation
Quoted 6 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +6
-0+void test_read_of_access_disabled_region_with_page_already_mapped (int *ptr, +u16 pkey) +{ +int ptr_contents; + + dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",
tools/testing/selftests/vm/protection_keys.c void test_read_of_access_disabled_region(int *ptr, u16 pkey) · line 1043
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +10
-0+ pkey, ptr); + ptr_contents =read_ptr (ptr); +dprintf1 ("reading ptr before disabling the read : %d\n" , + ptr_contents); +read_pkey_reg (); +pkey_access_deny (pkey); + ptr_contents =read_ptr (ptr); +dprintf1 ("*ptr: %d\n" , ptr_contents); +expected_pkey_fault (pkey); +}
Looks fine to me. I'm a bit surprised we didn't do this already, which is a good thing for this patch.
FWIW, if you took patches like this and put them first, you could probably get it merged now. Yes, I know it would mean redoing some of the later code move and rename ones.
Re: [PATCH v13 19/24] selftests/vm: associate key on a mapped page and detect access violation
Quoted 21 lines · Dave Hansen
Quoted 6 lines · Ram Pai
tools/testing/selftests/vm/protection_keys.c +6
-0+void test_read_of_access_disabled_region_with_page_already_mapped (int *ptr, +u16 pkey) +{ +int ptr_contents; + + dprintf1("disabling access to PKEY[%02d], doing read @ %p\n",tools/testing/selftests/vm/protection_keys.c void test_read_of_access_disabled_region(int *ptr, u16 pkey) · line 1043
Quoted 10 lines
tools/testing/selftests/vm/protection_keys.c +10
-0+ pkey, ptr); + ptr_contents =read_ptr (ptr); +dprintf1 ("reading ptr before disabling the read : %d\n" , + ptr_contents); +read_pkey_reg (); +pkey_access_deny (pkey); + ptr_contents =read_ptr (ptr); +dprintf1 ("*ptr: %d\n" , ptr_contents); +expected_pkey_fault (pkey); +}Looks fine to me. I'm a bit surprised we didn't do this already, which is a good thing for this patch.
Quoted 3 lines
FWIW, if you took patches like this and put them first, you could probably get it merged now. Yes, I know it would mean redoing some of the later code move and rename ones.
I have incorporated almost all of your comments. But there are some comments that take some effort to implement. Shall we get the patches merged in the current form? This code has been sitting out for a while.
In the current form its tested and works on powerpc and on x86, and incorporates about 95% of your suggestions. The rest I will take care as we go.
Signature · 1 line
Ram Pai
Re: [PATCH v13 19/24] selftests/vm: associate key on a mapped page and detect access violation
Quoted 7 lines · Ram Pai
I have incorporated almost all of your comments. But there are some comments that take some effort to implement. Shall we get the patches merged in the current form? This code has been sitting out for a while.
In the current form its tested and works on powerpc and on x86, and incorporates about 95% of your suggestions. The rest I will take care as we go.
What constitutes the remaining 5%?
Re: [PATCH v13 19/24] selftests/vm: associate key on a mapped page and detect access violation
Quoted 10 lines · Dave Hansen
Quoted 7 lines · Ram Pai
I have incorporated almost all of your comments. But there are some comments that take some effort to implement. Shall we get the patches merged in the current form? This code has been sitting out for a while.
In the current form its tested and works on powerpc and on x86, and incorporates about 95% of your suggestions. The rest I will take care as we go.
What constitutes the remaining 5%?
Mostly your comments on code-organization in the signal handler. There are still some #if defined(__i386__) ..... Can be cleaned up and abstracted further.
Also your questions on some of the code changes, the rationale for which is not obvious. Will help to spinkle in some descriptive comments there.
Have fixed up a lot of codying style issues. But there could till be a few that may spew warning by checkpatch.pl.
There are no functional issues AFAICT.
RP
PATCH v13 20/24 selftests/vm: associate key on a mapped page and detect write violation
In reply to Ram Paidetect write-violation on a page to which write-disabled key is associated much after the page is mapped.
--- tools/testing/selftests/vm/protection_keys.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/protection_keys.c +12 -0
@@ -1042,6 +1042,17 @@ void test_read_of_access_disabled_region_with_page_already_mapped(int *ptr,expected_pkey_fault (pkey); } +void test_write_of_write_disabled_region_with_page_already_mapped (int *ptr, +u16 pkey) +{ + *ptr = __LINE__; +dprintf1 ("disabling write access; after accessing the page, " +"to PKEY[%02d], doing write\n" , pkey); +pkey_write_deny (pkey); + *ptr = __LINE__; +expected_pkey_fault (pkey); +} +void test_write_of_write_disabled_region (int *ptr,u16 pkey) {dprintf1 ("disabling write access to PKEY[%02d], doing write\n" , pkey);
@@ -1422,6 +1433,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) test_read_of_access_disabled_region, test_read_of_access_disabled_region_with_page_already_mapped, test_write_of_write_disabled_region, + test_write_of_write_disabled_region_with_page_already_mapped, test_write_of_access_disabled_region, test_kernel_write_of_access_disabled_region, test_kernel_write_of_write_disabled_region,
Signature · 1 line
1.7.1
PATCH v13 21/24 selftests/vm: detect write violation on a mapped access-denied-key page
In reply to Ram Paidetect write-violation on a page to which access-disabled key is associated much after the page is mapped.
--- tools/testing/selftests/vm/protection_keys.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/protection_keys.c +13 -0
@@ -1067,6 +1067,18 @@ void test_write_of_access_disabled_region(int *ptr, u16 pkey) *ptr = __LINE__;expected_pkey_fault (pkey); } + +void test_write_of_access_disabled_region_with_page_already_mapped (int *ptr, +u16 pkey) +{ + *ptr = __LINE__; +dprintf1 ("disabling access; after accessing the page, " +" to PKEY[%02d], doing write\n" , pkey); +pkey_access_deny (pkey); + *ptr = __LINE__; +expected_pkey_fault (pkey); +} +void test_kernel_write_of_access_disabled_region (int *ptr,u16 pkey) {int ret;
@@ -1435,6 +1447,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) test_write_of_write_disabled_region, test_write_of_write_disabled_region_with_page_already_mapped, test_write_of_access_disabled_region, + test_write_of_access_disabled_region_with_page_already_mapped, test_kernel_write_of_access_disabled_region, test_kernel_write_of_write_disabled_region, test_kernel_gup_of_access_disabled_region,
Signature · 1 line
1.7.1
PATCH v13 22/24 selftests/vm: testcases must restore pkey-permissions
In reply to Ram PaiGenerally the signal handler restores the state of the pkey register before returning. However there are times when the read/write operation can legitamely fail without invoking the signal handler. Eg: A sys_read() operaton to a write-protected page should be disallowed. In such a case the state of the pkey register is not restored to its original state. The test case is responsible for restoring the key register state to its original value.
--- tools/testing/selftests/vm/protection_keys.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/protection_keys.c +5 -0
@@ -1011,6 +1011,7 @@ void test_read_of_write_disabled_region(int *ptr, u16 pkey) ptr_contents =read_ptr (ptr);dprintf1 ("*ptr: %d\n" , ptr_contents);dprintf1 ("\n" ); +pkey_write_allow (pkey); }void test_read_of_access_disabled_region (int *ptr,u16 pkey) {
@@ -1090,6 +1091,7 @@ void test_kernel_write_of_access_disabled_region(int *ptr, u16 pkey) ret =read (test_fd, ptr,1 );dprintf1 ("read ret: %d\n" , ret);pkey_assert (ret); +pkey_access_allow (pkey); }void test_kernel_write_of_write_disabled_region (int *ptr,u16 pkey) {
@@ -1102,6 +1104,7 @@ void test_kernel_write_of_write_disabled_region(int *ptr, u16 pkey)if (ret <0 && (DEBUG_LEVEL >0 ))perror ("verbose read result (OK for this to be bad)" );pkey_assert (ret); +pkey_write_allow (pkey); }void test_kernel_gup_of_access_disabled_region (int *ptr,u16 pkey)
@@ -1121,6 +1124,7 @@ void test_kernel_gup_of_access_disabled_region(int *ptr, u16 pkey) vmsplice_ret =vmsplice (pipe_fds[1 ], &iov,1 ,SPLICE_F_GIFT );dprintf1 ("vmsplice() ret: %d\n" , vmsplice_ret);pkey_assert (vmsplice_ret ==-1 ); +pkey_access_allow (pkey);close (pipe_fds[0 ]);close (pipe_fds[1 ]);
@@ -1141,6 +1145,7 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)if (DEBUG_LEVEL >0 )perror ("futex" );dprintf1 ("futex() ret: %d\n" , futex_ret); +pkey_write_allow (pkey); }/* Assumes that all pkeys other than 'pkey' are unallocated */
Signature · 1 line
1.7.1
Re: [PATCH v13 22/24] selftests/vm: testcases must restore pkey-permissions
Quoted 7 lines · Ram Pai
Generally the signal handler restores the state of the pkey register before returning. However there are times when the read/write operation can legitamely fail without invoking the signal handler. Eg: A
sys_read()operaton to a write-protected page should be disallowed. In such a case the state of the pkey register is not restored to its original state. The test case is responsible for restoring the key register state to its original value.
Seems fragile. Can't we just do this in common code? We could just loop through and restore the default permissions. That seems much more resistant to a bad test case.
Re: [PATCH v13 22/24] selftests/vm: testcases must restore pkey-permissions
Quoted 9 lines · Dave Hansen
Quoted 7 lines · Ram Pai
Generally the signal handler restores the state of the pkey register before returning. However there are times when the read/write operation can legitamely fail without invoking the signal handler. Eg: A
sys_read()operaton to a write-protected page should be disallowed. In such a case the state of the pkey register is not restored to its original state. The test case is responsible for restoring the key register state to its original value.
Quoted 3 lines
Seems fragile. Can't we just do this in common code? We could just loop through and restore the default permissions. That seems much more resistant to a bad test case.
Yes. done. fixed it the way you suggested in the new version.
Signature · 1 line
Ram Pai
PATCH v13 23/24 selftests/vm: sub-page allocator
In reply to Ram Paiintroduce a new allocator that allocates 4k hardware-pages to back 64k linux-page. This allocator is only applicable on powerpc.
--- tools/testing/selftests/vm/pkey-helpers.h | 6 ++++++ tools/testing/selftests/vm/pkey-powerpc.h | 25 +++++++++++++++++++++++++ tools/testing/selftests/vm/pkey-x86.h | 5 +++++ tools/testing/selftests/vm/protection_keys.c | 1 + 4 files changed, 37 insertions(+), 0 deletions(-)
tools/testing/selftests/vm/pkey-helpers.h +6 -0
@@ -28,6 +28,9 @@extern int dprint_in_signal;extern char dprint_in_signal_buffer[DPRINT_IN_SIGNAL_BUF_SIZE ]; +extern int test_nr; +extern int iteration_nr; +#ifdef __GNUC__ __attribute__((format(printf, 1, 2)))#endif
@@ -78,6 +81,9 @@ static inline void sigsafe_printf(const char *format, ...)void expected_pkey_fault (int pkey);int sys_pkey_alloc (unsigned long flags,u64 init_val);int sys_pkey_free (unsigned long pkey); +int mprotect_pkey (void *ptr,size_t size,unsigned long orig_prot, +unsigned long pkey); +void record_pkey_malloc (void *ptr,long size,int prot);#if defined(__i386__) || defined(__x86_64__)/* arch */ #include "pkey-x86.h"
tools/testing/selftests/vm/pkey-powerpc.h +25 -0
@@ -98,4 +98,29 @@ void expect_fault_on_read_execonly_key(void *p1, u16 pkey)/* 8-bytes of instruction * 16384bytes = 1 page */ #define __page_o_noops () asm(".rept 16384 ; nop; .endr") +void *malloc_pkey_with_mprotect_subpage (long size,int prot,u16 pkey) +{ +void *ptr; +int ret; + +dprintf1 ("doing %s(size=%ld, prot=0x%x, pkey=%d)\n" , __func__, + size, prot, pkey); +pkey_assert (pkey <NR_PKEYS ); + ptr =mmap (NULL , size, prot,MAP_ANONYMOUS |MAP_PRIVATE ,-1 ,0 ); +pkey_assert (ptr != (void *)-1 ); + + ret =syscall (__NR_subpage_prot, ptr, size,NULL ); +if (ret) { +perror ("subpage_perm" ); +return PTR_ERR_ENOTSUP ; + } + + ret =mprotect_pkey ((void *)ptr,PAGE_SIZE , prot, pkey); +pkey_assert (!ret); +record_pkey_malloc (ptr, size, prot); + +dprintf1 ("%s() for pkey %d @ %p\n" , __func__, pkey, ptr); +return ptr; +} +#endif /* _PKEYS_POWERPC_H */
tools/testing/selftests/vm/pkey-x86.h +5 -0
@@ -176,4 +176,9 @@ void expect_fault_on_read_execonly_key(void *p1, u16 pkey) expected_pkey_fault(pkey); } +void *malloc_pkey_with_mprotect_subpage (long size,int prot,u16 pkey) +{ +return PTR_ERR_ENOTSUP ; +} +#endif /* _PKEYS_X86_H */
tools/testing/selftests/vm/protection_keys.c +1 -0
@@ -887,6 +887,7 @@ void setup_hugetlbfs(void)void *(*pkey_malloc[])(long size,int prot,u16 pkey) = { malloc_pkey_with_mprotect, + malloc_pkey_with_mprotect_subpage, malloc_pkey_anon_huge, malloc_pkey_hugetlb/* can not do direct with the pkey_mprotect() API:
Signature · 1 line
1.7.1
PATCH v13 24/24 selftests/vm: test correct behavior of pkey-0
In reply to Ram PaiEnsure pkey-0 is allocated on start. Ensure pkey-0 can be attached dynamically in various modes, without failures. Ensure pkey-0 can be freed and allocated.
--- tools/testing/selftests/vm/protection_keys.c | 66 +++++++++++++++++++++++++- 1 files changed, 64 insertions(+), 2 deletions(-)
tools/testing/selftests/vm/protection_keys.c +64 -2
@@ -1003,6 +1003,67 @@ void close_test_fds(void)return *ptr; } +void test_pkey_alloc_free_attach_pkey0 (int *ptr,u16 pkey) +{ +int i, err; +int max_nr_pkey_allocs; +int alloced_pkeys[NR_PKEYS ]; +int nr_alloced =0 ; +int newpkey; +long size; + +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 ; + + +/* allocate every possible key and make sure key-0 never got allocated */ + max_nr_pkey_allocs =NR_PKEYS ; +for (i =0 ; i < max_nr_pkey_allocs; i++) { +int new_pkey =alloc_pkey (); +assert (new_pkey !=0 ); + +if (new_pkey <0 ) +break ; + alloced_pkeys[nr_alloced++] = new_pkey; + } +/* free all the allocated keys */ +for (i =0 ; i < nr_alloced; i++) { +int free_ret; + +if (!alloced_pkeys[i]) +continue ; + free_ret =sys_pkey_free (alloced_pkeys[i]); +pkey_assert (!free_ret); + } + +/* attach key-0 in various modes */ + err =sys_mprotect_pkey (ptr, size,PROT_READ ,0 ); +pkey_assert (!err); + err =sys_mprotect_pkey (ptr, size,PROT_WRITE ,0 ); +pkey_assert (!err); + err =sys_mprotect_pkey (ptr, size,PROT_EXEC ,0 ); +pkey_assert (!err); + err =sys_mprotect_pkey (ptr, size,PROT_READ |PROT_WRITE ,0 ); +pkey_assert (!err); + err =sys_mprotect_pkey (ptr, size,PROT_READ |PROT_WRITE |PROT_EXEC ,0 ); +pkey_assert (!err); + +/* free key-0 */ + err =sys_pkey_free (0 ); +pkey_assert (!err); + + newpkey =sys_pkey_alloc (0 ,0x0 ); +assert (newpkey ==0 ); +} +void test_read_of_write_disabled_region (int *ptr,u16 pkey) {int ptr_contents;
@@ -1153,10 +1214,10 @@ void test_kernel_gup_write_to_write_disabled_region(int *ptr, u16 pkey)void test_pkey_syscalls_on_non_allocated_pkey (int *ptr,u16 pkey) {int err; -int i =get_start_key (); +int i;/* Note: 0 is the default pkey, so don't mess with it */ -for (; i <NR_PKEYS ; i++) { +for (i=1 ; i <NR_PKEYS ; i++) {if (pkey == i)continue ;
@@ -1465,6 +1526,7 @@ void test_mprotect_pkey_on_unsupported_cpu(int *ptr, u16 pkey) test_pkey_syscalls_on_non_allocated_pkey, test_pkey_syscalls_bad_args, test_pkey_alloc_exhaust, + test_pkey_alloc_free_attach_pkey0, };void run_tests_once (void )
Signature · 1 line
1.7.1
Re: [PATCH v13 24/24] selftests/vm: test correct behavior of pkey-0
Quoted 3 lines · Ram Pai
Ensure pkey-0 is allocated on start. Ensure pkey-0 can be attached dynamically in various modes, without failures. Ensure pkey-0 can be freed and allocated.
I like this. Looks very useful.
Quoted 4 lines · Ram Pai
Test
----
Verified for correctness on powerpc. Need help verifying on x86. Compiles on x86.
It breaks make in tools/testing/selftests/x86:
make: *** No rule to make target protection_keys.c', needed by /home/linux/tools/testing/selftests/x86/protection_keys_64'. Stop.
The generic implementation no longer builds 32-bit binaries. Is this the intent?
It's possible to build 32-bit binaries with “make CC='gcc -m32'”, so perhaps this is good enough?
But with that, I get a warning:
protection_keys.c: In function ‘dump_mem’:
protection_keys.c:172:3: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat=]
dprintf1("dump[%03d][@%p]: %016lx\n", i, ptr, *ptr);
^
I suppose you could use %016llx and add a cast to unsigned long long to fix this.
Anyway, both the 32-bit and 64-bit tests fail here:
assert() at protection_keys.c::943 test_nr: 12 iteration: 1 running abort_hooks()...
I've yet checked what causes this. It's with the kernel headers from 4.17, but with other userspace headers based on glibc 2.17. I hope to look into this some more before the weekend, but I eventually have to return the test machine to the pool.
Thanks,
Florian
Quoted 10 lines · Florian Weimer
Quoted 4 lines · Ram Pai
Test
----
Verified for correctness on powerpc. Need help verifying on x86. Compiles on x86.It breaks make in
tools/testing/selftests/x86:make: *** No rule to make target
protection_keys.c', needed by/home/linux/tools/testing/selftests/x86/protection_keys_64'. Stop.
Ah.. it has to be taken out from the Makefile of/home/linux/tools/testing/selftests/x86/
The sources have been moved to /home/linux/tools/testing/selftests/mm/
Quoted 3 lines
The generic implementation no longer builds 32-bit binaries. Is this the intent?
No. But building it 32-bit after moving it to a the new directory needs some special code in the Makefile.
Quoted 3 lines
It's possible to build 32-bit binaries with “make CC='gcc -m32'”, so perhaps this is good enough?
Dave Hansen did mention it, but he did not complain too much. So I kept quite.
Quoted 7 lines
But with that, I get a warning:
protection_keys.c: In function ‘dump_mem’:
protection_keys.c:172:3: warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Wformat=]
Quoted 5 lines
dprintf1("dump[%03d][@%p]: %016lx\n", i, ptr, *ptr); ^I suppose you could use %016llx and add a cast to unsigned long long to fix this.
yes.
Quoted 10 lines
Anyway, both the 32-bit and 64-bit tests fail here:
assert()at protection_keys.c::943test_nr: 12 iteration: 1 runningabort_hooks()...I've yet checked what causes this. It's with the kernel headers from 4.17, but with other userspace headers based on glibc 2.17. I hope to look into this some more before the weekend, but I eventually have to return the test machine to the pool.
I wish I could get a x86 machine which could do memory keys. Had a AWS instance, but struggled to boot my kernel. Can't get to the console... gave up. If someone can give me a ready-made machine with support for memkeys, I can quickly fix all the outstanding x86 issues. But if someone can just fix it for me, .... ;)
RP