Re: [PATCH bpf-next v4 1/7] mm: Add copy_remote_mm_str()

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Mon, Sep 07, 2026 at 04:41:01PM +0300, Anastasios Papagiannis wrote:
> > We have this check in copy_remote_vm_str(). Why are we performing the check now
> > twice?
>
> > It should either go only into __copy_remote_mm_str(), or if there a reason to
> > have it before get_task_mm(), it should go into copy_remote_mm_str(). Same
> > applies to the memory.c case.
>
> Yes, this makes sense. I will fix that.
>
> > What's more annoying is that both implementations of copy_remote_vm_str() are
> > identical, and both implementations of copy_remote_mm_str() are nearly identical
> > (just dropping the gup_flags for nommu). I'd like to avoid duplicating code for
> > nommu.
>
> > If we could export __copy_remote_vm_str(mm, addr, buf, len, gup_flags) for both
> > cases, we could instead provide a single implementation for copy_remote_vm_str()
> > and copy_remote_mm_str() e.g., in mm.h? (I'd prefer somewhere else, but we don't
> > seem to have a good git for memory.c + nommu.c shared stuff)
>
> Another idea can be:
> mm/memory.c: MMU implementation of __copy_remote_mm_str()
> mm/nommu.c: NOMMU implementation of __copy_remote_mm_str()
> mm/internal.h: declaration of __copy_remote_mm_str()
> include/linux/mm.h: declaration of copy_remote_mm_str() and  copy_remote_vm_str()
> mm/util.c: shared implementation for copy_remote_mm_str() and copy_remote_vm_str()

I mean copy_remote_vm_str() is tiny, so maybe just inline it in mm.h?

get_task_mm() is available from include/linux/sched/mm.h anyway so it's not a
problem to use that there.

>
> This allows us to remove the duplicate code. Does this sound reasonable?
>
> > Now, that's also not completely nice, as I don't want us to EXPORT
> > __copy_remote_vm_str() ... given that these functions are "#ifdef
> > CONFIG_BPF_SYSCALL" could we EXPORT_SYMBOL_FOR_MODULES?
>
> Now copy_remote_vm_str() is EXPORT_SYMBOL_GPL. In this series, we use
> copy_remote_mm_str() without the need to export that. Why do we need to
> consider exporting __copy_remote_vm_str()?

I don't think that's a problem, because all copy_remote_vm_str() is is:

- get_task_mm() (already GPL exported)
- invokes __copy_remote_vm_str()

It already requires that the caller has pinned mm, and I guess the one key
difference is you can't pass some stupid parameter like NULL mm and have it
break.

But if you're kernel code you can NULL ptr deref without anybody's help so :)

The other concern would be accessing a remote mm but... that's literally the
whole point of the function and we already export that.

I guess the other thing is mm copy_remote_vm_str() is only available if
CONFIG_BPF_SYSCALL is enabled but that's pretty much any sensible kernel config
so meh doesn't matter really.

>
> --
> Thanks,
> -Anastasios

--
Cheers, Lorenzo