powerpc: do not allow userspace to modify execute-only pkey
PATCH v2 1/1 powerpc: do not allow userspace to modify execute-only pkey
When mprotect(....,PROT_EXEC) is called, the kernel allocates a execute-only pkey and associates the pkey with the given address space. The permission of this key should not be modifiable from userspace. However a bug in the current implementation lets the permissions on the key modifiable from userspace.
Whenever a key is allocated through mm_pkey_alloc(), the kernel programs the UAMOR register to allow userspace to change permissions on the key. This is fine for keys explicitly allocated through the sys_pkey_alloc(). But for execute-only pkey, it must be disallowed. Restructured the code to fix the bug.
---
History:
v2: Thiago noticed a bug -- __execute_only_pkey() will always fail
since it calls is_pkey_enabled() which always returns false for execute_only key. is_pkey_enabled() returns false because UAMOR bit for the execute_only key is and never be set. Fixed it.
arch/powerpc/include/asm/pkeys.h | 24 ++++------------ arch/powerpc/mm/pkeys.c | 57 ++++++++++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 29 deletions(-)
arch/powerpc/include/asm/pkeys.h +6 -18
@@ -113,6 +113,8 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)extern void __arch_activate_pkey (int pkey);extern void __arch_deactivate_pkey (int pkey); +extern int __mm_pkey_alloc (struct mm_struct *mm); +/* * Returns a positive, 5-bit key on success, or -1 on failure. * Relies on the mmap_sem to protect against concurrency in mm_pkey_alloc() and
@@ -120,29 +122,14 @@ static inline bool mm_pkey_is_allocated(struct mm_struct *mm, int pkey)*/ static inline int mm_pkey_alloc (struct mm_struct *mm) { -/* -* Note: this is the one and only place we make sure that the pkey is -* valid as far as the hardware is concerned. The rest of the kernel -* trusts that only good, valid pkeys come out of here. -*/ -u32 all_pkeys_mask = (u32)(~(0x0 ));int ret;if (static_branch_likely (&pkey_disabled))return -1 ; + ret =__mm_pkey_alloc (mm);/* -* Are we out of pkeys? We must handle this specially because ffz() -* behavior is undefined if there are no zeros. -*/ -if (mm_pkey_allocation_map (mm) == all_pkeys_mask) -return -1 ; - - ret =ffz ((u32 )mm_pkey_allocation_map (mm)); -__mm_pkey_allocated (mm, ret); - -/* -* Enable the key in the hardware +* Enable userspace to modify the key permissions. */ if (ret >0 )__arch_activate_pkey (ret);
@@ -158,7 +145,8 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)return -EINVAL ;/* -* Disable the key in the hardware +* Reset the key and disable userspace +* from modifying the key permissions. */ __arch_deactivate_pkey (pkey);__mm_pkey_free (mm, pkey);
arch/powerpc/mm/pkeys.c +46 -11
98 lines not on this page · raw message
Signature · 1 line
1.7.1