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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/7/26 18:52, Anastasios Papagiannis wrote:
> copy_remote_vm_str() gets the target address space from a struct
> task_struct. This does not work for an address space that exists but is
> not yet associated with a task_struct, such as the mm held by struct
> linux_binprm during exec.
> 
> Add copy_remote_mm_str(), which operates directly on a struct mm_struct.
> 
> Use a common internal interface for the MMU and NOMMU implementations
> and define both public wrappers in mm/util.c. Preserve the existing
> copy_remote_vm_str() behavior, including handling zero-length requests
> before acquiring the task's mm.
> 
> Signed-off-by: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>
> ---
>  include/linux/mm.h |  2 ++
>  mm/internal.h      |  5 ++++
>  mm/memory.c        | 41 ++----------------------------
>  mm/nommu.c         | 41 ++----------------------------
>  mm/util.c          | 62 ++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 73 insertions(+), 78 deletions(-)

Almost looks like a cleanup now ;)

> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index dd09c438fa23..d5bde1f71a97 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3326,6 +3326,8 @@ extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
>  		void *buf, int len, unsigned int gup_flags);
>  
>  #ifdef CONFIG_BPF_SYSCALL
> +extern int copy_remote_mm_str(struct mm_struct *mm, unsigned long addr,
> +			      void *buf, int len, unsigned int gup_flags);
>  extern int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr,
>  			      void *buf, int len, unsigned int gup_flags);

Ah, no new "extern" and best to drop the "extern" from the other involved
function while at it.

Also, you can just drop the ifdef; the linker will complain either way later.

>  #endif
> diff --git a/mm/internal.h b/mm/internal.h
> index 38b1165212c9..8264a346d18a 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -25,6 +25,11 @@
>  struct folio_batch;
>  struct hstate;
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +int __copy_remote_mm_str(struct mm_struct *mm, unsigned long addr,
> +			 void *buf, int len, unsigned int gup_flags);
> +#endif
> +

Same here, the linker will complain, so no need for the ifdef.



Apart from that LGTM, thanks!

-- 
Cheers,

David