Re: [PATCH] powerpc/pkeys: copy pkey-tracking-information at fork()

[Ram Pai <linuxram@us.ibm.com>]

On Fri, Dec 21, 2018 at 12:19:13AM +1100, Michael Ellerman wrote:
> Hi Ram,
> 
> Thanks for fixing this.
> 
> Ram Pai <linuxram@us.ibm.com> writes:
> > 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;
> > +}
> 
> Shouldn't this check if pkeys are actually in use?

Yes. That will make it better. However not checking it, will not hurt. Just that
it will do some unnecessary copying.

Will spin out a new patch with the fix, and send it your way.
RP


> 
> eg:
> 
> diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
> index cf87dddefbdc..587807763737 100644
> --- a/arch/powerpc/mm/pkeys.c
> +++ b/arch/powerpc/mm/pkeys.c
> @@ -418,6 +418,9 @@ bool arch_vma_access_permitted(struct vm_area_struct *vma, bool write,
> 
>  void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm)
>  {
> +	if (static_branch_likely(&pkey_disabled))
> +		return;
> +
>  	/* 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;
> 
> 
> Ideally we'd actually do it in the inline so that the function call to
> arch_dup_pkeys() can be avoided. But it looks like header dependencies
> might make that hard.
> 
> cheers

-- 
Ram Pai