Hello,
Ram Pai <linux...@us.ibm.com> writes: > Key 2 is preallocated and reserved for execute-only key. In rare > cases if key-2 is unavailable, mprotect(PROT_EXEC) will behave > incorrectly. NOTE: mprotect(PROT_EXEC) uses execute-only key. > > Ensure key 2 is available for preallocation before reserving it for > execute_only purpose. Problem noticed by Michael Ellermen. Since "powerpc/pkeys: Preallocate execute-only key" isn't upstream yet, this patch could be squashed into it. > Signed-off-by: Ram Pai <linux...@us.ibm.com> > --- > arch/powerpc/mm/pkeys.c | 14 +++++++++----- > 1 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c > index cec990c..0b03914 100644 > --- a/arch/powerpc/mm/pkeys.c > +++ b/arch/powerpc/mm/pkeys.c > @@ -19,6 +19,7 @@ > u64 pkey_amr_mask; /* Bits in AMR not to be touched */ > u64 pkey_iamr_mask; /* Bits in AMR not to be touched */ > u64 pkey_uamor_mask; /* Bits in UMOR not to be touched */ > +int execute_only_key = 2; > > #define AMR_BITS_PER_PKEY 2 > #define AMR_RD_BIT 0x1UL > @@ -26,7 +27,6 @@ > #define IAMR_EX_BIT 0x1UL > #define PKEY_REG_BITS (sizeof(u64)*8) > #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY)) > -#define EXECUTE_ONLY_KEY 2 > > static void scan_pkey_feature(void) > { > @@ -122,8 +122,12 @@ int pkey_initialize(void) > #else > os_reserved = 0; > #endif > + > + if ((pkeys_total - os_reserved) <= execute_only_key) > + execute_only_key = -1; > + > /* Bits are in LE format. */ > - reserved_allocation_mask = (0x1 << 1) | (0x1 << EXECUTE_ONLY_KEY); > + reserved_allocation_mask = (0x1 << 1) | (0x1 << execute_only_key); My understanding is that left-shifting by a negative amount is undefined behavior in C. A quick test tells me that at least on the couple of machines I tested, 1 < -1 = 0. Does GCC guarantee that behavior? If so, a comment pointing this out would make this less confusing. > initial_allocation_mask = reserved_allocation_mask | (0x1 << PKEY_0); > > /* register mask is in BE format */ > @@ -132,11 +136,11 @@ int pkey_initialize(void) > > pkey_iamr_mask = ~0x0ul; > pkey_iamr_mask &= ~(0x3ul << pkeyshift(PKEY_0)); > - pkey_iamr_mask &= ~(0x3ul << pkeyshift(EXECUTE_ONLY_KEY)); > + pkey_iamr_mask &= ~(0x3ul << pkeyshift(execute_only_key)); > > pkey_uamor_mask = ~0x0ul; > pkey_uamor_mask &= ~(0x3ul << pkeyshift(PKEY_0)); > - pkey_uamor_mask &= ~(0x3ul << pkeyshift(EXECUTE_ONLY_KEY)); > + pkey_uamor_mask &= ~(0x3ul << pkeyshift(execute_only_key)); Here the behaviour is undefined in C as well, given that pkeyshift(-1) = 64, which is the total number of bits in the left operand. Does GCC guarantee that the result will be 0 here as well? -- Thiago Jung Bauermann IBM Linux Technology Center