Re: [RFC bpf-next 3/9] mm: Expose locked mprotect operation

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Oct 22, 2026 at 11:25:31AM +0100, Lorenzo Stoakes (ARM) wrote:
> On Wed, Oct 21, 2026 at 09:57:52PM +0200, Jiri Olsa wrote:
> > The bpf_user_page_exec() in following patch needs to change page
> > protections while holding current->mm's mmap write lock.
> >
> > Exposing the do_mprotect_pkey_locked helper for that.
> 
> Err, we'll need some more justification than this for exposing internal mm
> logic to the rest of the kernel please.
> 
> Note that you're already allowing any caller to bypass
> CONFIG_ARCH_HAS_PKEYS here which is immediately a broken thing.
> 
> >
> > Assisted-by: Codex:GPT-6
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> It's a no to this in its current form.

yes I suspected it'd be a problem, but as you mentioned
in the other reply, Alexei's proposal takes care of that

thanks,
jirka

> 
> > ---
> >  include/linux/mm.h |  3 +++
> >  mm/mprotect.c      | 20 +++++++++++++++-----
> >  2 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 63f40e615754..47e853093601 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -3435,6 +3435,9 @@ extern long change_protection(struct mmu_gather *tlb,
> >  extern int mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
> >  	  struct vm_area_struct *vma, struct vm_area_struct **pprev,
> >  	  unsigned long start, unsigned long end, vm_flags_t newflags);
> > +/* Caller must hold current->mm's mmap write lock. */
> > +int do_mprotect_pkey_locked(unsigned long start, size_t len,
> > +			    unsigned long prot, int pkey);
> >
> >  /*
> >   * doesn't attempt to fault and will return short.
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 2888ee638d87..8d800830d3e1 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -868,8 +868,8 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
> >  /*
> >   * pkey==-1 when doing a legacy mprotect()
> >   */
> > -static int do_mprotect_pkey(unsigned long start, size_t len,
> > -		unsigned long prot, int pkey)
> > +int do_mprotect_pkey_locked(unsigned long start, size_t len,
> > +			    unsigned long prot, int pkey)
> >  {
> >  	unsigned long nstart, end, tmp, reqprot;
> >  	struct vm_area_struct *vma, *prev;
> > @@ -899,8 +899,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
> >
> >  	reqprot = prot;
> >
> > -	if (mmap_write_lock_killable(current->mm))
> > -		return -EINTR;
> > +	mmap_assert_write_locked(current->mm);
> >
> >  	/*
> >  	 * If userspace did not allocate the pkey, do not let
> > @@ -1013,10 +1012,21 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
> >  		error = -ENOMEM;
> >
> >  out:
> > -	mmap_write_unlock(current->mm);
> >  	return error;
> >  }
> >
> > +static int do_mprotect_pkey(unsigned long start, size_t len,
> > +			    unsigned long prot, int pkey)
> > +{
> > +	int ret;
> > +
> > +	if (mmap_write_lock_killable(current->mm))
> > +		return -EINTR;
> > +	ret = do_mprotect_pkey_locked(start, len, prot, pkey);
> > +	mmap_write_unlock(current->mm);
> > +	return ret;
> > +}
> > +
> >  SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
> >  		unsigned long, prot)
> >  {
> > --
> > 2.54.0
> >
> 
> --
> Cheers, Lorenzo