lkml.org 
[lkml]   [2022]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] pkeys: Make pkey unsigned in arch_set_user_pkey_access()
Date
ira.weiny@intel.com writes:

> From: Ira Weiny <ira.weiny@intel.com>
>
> The WARN_ON check in arch_set_user_pkey_access() in the x86 architecture
> fails to check for an invalid negative value.
>
> A simple check for less than 0 would fix this issue however, in the call
> stack below arch_set_user_pkey_access() the pkey should never be
> negative on any architecture. It is always best to use correct types
> when possible. x86 only supports 16 keys while ppc supports 32, u8 is
> therefore large enough for all current architectures and likely those in
> the future.

Should we do that as a separate patch? ie, now convert the variable to
unsigned int and later switch all the variables to u8? because what we
now have is confusing.

static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
unsigned long pkey)
static inline u64 pkey_to_vmflag_bits(u16 pkey)



>
> Change the type of the pkey passed to arch_set_user_pkey_access() to u8.
>
> To: Dave Hansen <dave.hansen@linux.intel.com>
> To: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> ---
> arch/powerpc/include/asm/pkeys.h | 4 ++--
> arch/powerpc/mm/book3s64/pkeys.c | 2 +-
> arch/x86/include/asm/pkeys.h | 4 ++--
> arch/x86/kernel/fpu/xstate.c | 2 +-
> include/linux/pkeys.h | 2 +-
> 5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
> index 59a2c7dbc78f..e70615a1da9b 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -143,9 +143,9 @@ static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
> return __arch_override_mprotect_pkey(vma, prot, pkey);
> }
>
> -extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +extern int __arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val);


> -static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +static inline int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val)
> {
> if (!mmu_has_feature(MMU_FTR_PKEY))
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 753e62ba67af..c048467669df 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -333,7 +333,7 @@ static inline void init_iamr(int pkey, u8 init_bits)
> * Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
> * specified in @init_val.
> */
> -int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +int __arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val)
> {
> u64 new_amr_bits = 0x0ul;
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index 5292e6dfe2a7..48efb81f6cc6 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -9,7 +9,7 @@
> */
> #define arch_max_pkey() (cpu_feature_enabled(X86_FEATURE_OSPKE) ? 16 : 1)
>
> -extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +extern int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val);
>
> static inline bool arch_pkeys_enabled(void)
> @@ -115,7 +115,7 @@ int mm_pkey_free(struct mm_struct *mm, int pkey)
> return 0;
> }
>
> -extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +extern int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val);
>
> static inline int vma_pkey(struct vm_area_struct *vma)
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 7c7824ae7862..db511bec57e5 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1068,7 +1068,7 @@ void *get_xsave_addr(struct xregs_state *xsave, int xfeature_nr)
> * This will go out and modify PKRU register to set the access
> * rights for @pkey to @init_val.
> */
> -int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val)
> {
> u32 old_pkru, new_pkru_bits = 0;
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 86be8bf27b41..aa40ed2fb0fc 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -35,7 +35,7 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> return -EINVAL;
> }
>
> -static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
> +static inline int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
> unsigned long init_val)
> {
> return 0;
> --
> 2.35.1

\
 
 \ /
  Last update: 2022-03-07 08:02    [W:0.050 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site