Ram Pai <linux...@us.ibm.com> writes:
> At fork(), the pkey tracking information is not copied over > to the mm_struct of the child. This can cause the child to erroneously > allocate keys that were already allocated. Any allocated execute-only > key is lost aswell. > > Add code; called by dup_mmap(), to copy the pkey state from > parent to child explicitly. > > This problem was originally found by Dave Hansen on x86, which turns > out to be a problem on powerpc aswell. > > Testing: > Passes the new selftest added by Dave Hansen. I have temporarily > captured that selftest at > https://github.com/rampai/memorykeys/ -b memkey.v15-rc6 > > Signed-off-by: Ram Pai <linux...@us.ibm.com> > --- > arch/powerpc/include/asm/mmu_context.h | 15 +++++++++------ > arch/powerpc/mm/pkeys.c | 7 +++++++ > 2 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/arch/powerpc/include/asm/mmu_context.h > b/arch/powerpc/include/asm/mmu_context.h > index 0381394..cb16146 100644 > --- a/arch/powerpc/include/asm/mmu_context.h > +++ b/arch/powerpc/include/asm/mmu_context.h > @@ -217,12 +217,6 @@ static inline void enter_lazy_tlb(struct mm_struct *mm, > #endif > } > > -static inline int arch_dup_mmap(struct mm_struct *oldmm, > - struct mm_struct *mm) > -{ > - return 0; > -} > - > #ifndef CONFIG_PPC_BOOK3S_64 > static inline void arch_exit_mmap(struct mm_struct *mm) > { > @@ -247,6 +241,7 @@ static inline void arch_bprm_mm_init(struct mm_struct *mm, > #ifdef CONFIG_PPC_MEM_KEYS > bool arch_vma_access_permitted(struct vm_area_struct *vma, bool write, > bool execute, bool foreign); > +void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm); > #else /* CONFIG_PPC_MEM_KEYS */ > static inline bool arch_vma_access_permitted(struct vm_area_struct *vma, > bool write, bool execute, bool foreign) > @@ -259,6 +254,7 @@ static inline bool arch_vma_access_permitted(struct > vm_area_struct *vma, > #define thread_pkey_regs_save(thread) > #define thread_pkey_regs_restore(new_thread, old_thread) > #define thread_pkey_regs_init(thread) > +#define arch_dup_pkeys(oldmm, mm) > > static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) > { > @@ -267,5 +263,12 @@ static inline u64 pte_to_hpte_pkey_bits(u64 pteflags) > > #endif /* CONFIG_PPC_MEM_KEYS */ > > +static inline int arch_dup_mmap(struct mm_struct *oldmm, > + struct mm_struct *mm) > +{ > + arch_dup_pkeys(oldmm, mm); > + return 0; > +} > + > #endif /* __KERNEL__ */ > #endif /* __ASM_POWERPC_MMU_CONTEXT_H */ > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c > index b271b28..5d65c47 100644 > --- a/arch/powerpc/mm/pkeys.c > +++ b/arch/powerpc/mm/pkeys.c > @@ -414,3 +414,10 @@ bool arch_vma_access_permitted(struct vm_area_struct > *vma, bool write, > > return pkey_access_permitted(vma_pkey(vma), write, execute); > } > + > +void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm) > +{ > + /* Duplicate the oldmm pkey state in mm: */ > + mm_pkey_allocation_map(mm) = mm_pkey_allocation_map(oldmm); > + mm->context.execute_only_pkey = oldmm->context.execute_only_pkey; > +} This function is small and perhaps could have been a static inline in <asm/mmu_context.h>, but arch_dup_mmap() doesn't seem to be in a hot path so the segregation of implementation details to pkeys.c is nice from an organization point of view. Reviewed-by: Thiago Jung Bauermann <bauer...@linux.ibm.com> -- Thiago Jung Bauermann IBM Linux Technology Center