From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DCA1C550DB5; Tue, 8 Sep 2026 13:16:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788873401; cv=none; b=ekwIqPaPwWFROJhIv4HKizjVxJ8+UwWSXlPb0pWP4un5AnGD9Rh43oo7dBDNPoi1bd4nkMKEnMqThjDXmvzJsRmVYmR4XU/FRuc1sKooFohzQr46QFd+Hb9Zl6oIiT2vpquH8PZidtVqXdxhOsWt1XeLCdE1o45nZjMYlfFJmag= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788873401; c=relaxed/simple; bh=rw6q01fV3rH8LWshetnCVReEFs0SL+sAvVq67LFYUaY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=mIOBxQYBXxRJp8a8Fe20NGMNUVLvyLb1JNJMBzqHWxqKtpx7+UK0Qb6MOav69Toyj1eLBuTzqdoNGOvaMTydNILMsJUWMVaFOk1R44QvHrVNS2h0ta8ORB/7eYPY4W9/DJyM+UU5dUhKjoreaDrzhO3li2rz1jfMKowyr5q1R2I= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=oqeTVtVP; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="oqeTVtVP" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA0B31F00A3A; Tue, 8 Sep 2026 13:16:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1788873399; bh=eGoipUCnVkQSnrYlG4JuuefvC9x1Ssr7yfDjVV3fi44=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=oqeTVtVP8J4gmkOV/vgaJdwrdFmePsueOXneNkP0K3ZCZxNy/XbNk+Mz3u8yPvv8B rSJcvMVytF2Wex2GWEtxw3Jmo/U6steSK88MCOpPV2qzOXxtFldeoPnZu/oJdcDjjM mhW11GCOVUtyhlrgmqkMBozE9lYJzkTafjcL7YGglsbBZ90n9rZK1JNJBFwQBLyX3i laZwDs0zTiur85XZaUXHRw3RmT77Vb9vNnQ7FsJYwfisjB5J6eGu5yI97mQ74KEz+f 9Bj/yI275o/CX6460g5dfvsVUaGHS9oLnUWg3xP8vYkXlOQL1RZnt0vrBDmJZTYTvW htwTGDeZI9OXQ== Date: Tue, 8 Sep 2026 14:16:31 +0100 From: "Lorenzo Stoakes (ARM)" To: Anastasios Papagiannis Cc: bpf@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, david@kernel.org, akpm@linux-foundation.org, andrii@kernel.org, ast@kernel.org, brauner@kernel.org, daniel@iogearbox.net, eddyz87@gmail.com, kpsingh@kernel.org, matt@bobrowski.net, memxor@gmail.com, song@kernel.org, sun.jian.kdev@gmail.com, utilityemal77@gmail.com, viro@zeniv.linux.org.uk Subject: Re: [PATCH bpf-next v5 1/7] mm: Add copy_remote_mm_str() Message-ID: References: <20260907165220.52431-1-tasos.papagiannnis@gmail.com> <20260907165220.52431-2-tasos.papagiannnis@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260907165220.52431-2-tasos.papagiannnis@gmail.com> On Mon, Sep 07, 2026 at 07:52:14PM +0300, 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 OK actually doing it this way means we don't have to worry about __copy_remote_mm_str() at all since we abstract it with copy_remote_mm_str(). With David's comments addressed LGTM so: Acked-by: Lorenzo Stoakes (ARM) > --- > 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(-) > > 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); > #endif Same comments as David :>) > 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 > + > struct huge_bootmem_page { > struct list_head list; > struct hstate *hstate; > diff --git a/mm/memory.c b/mm/memory.c > index 8b0c2c735d3d..fe2f5e988fb9 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -7331,8 +7331,8 @@ EXPORT_SYMBOL_GPL(access_process_vm); > * Copy a string from another process's address space as given in mm. > * If there is any error return -EFAULT. > */ > -static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > - void *buf, int len, unsigned int gup_flags) > +int __copy_remote_mm_str(struct mm_struct *mm, unsigned long addr, > + void *buf, int len, unsigned int gup_flags) > { > void *old_buf = buf; > int err = 0; > @@ -7407,43 +7407,6 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > return err; > return buf - old_buf; > } > - > -/** > - * copy_remote_vm_str - copy a string from another process's address space. > - * @tsk: the task of the target address space > - * @addr: start address to read from > - * @buf: destination buffer > - * @len: number of bytes to copy > - * @gup_flags: flags modifying lookup behaviour > - * > - * The caller must hold a reference on @mm. > - * > - * Return: number of bytes copied from @addr (source) to @buf (destination); > - * not including the trailing NUL. Always guaranteed to leave NUL-terminated > - * buffer. On any error, return -EFAULT. > - */ > -int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr, > - void *buf, int len, unsigned int gup_flags) > -{ > - struct mm_struct *mm; > - int ret; > - > - if (unlikely(len == 0)) > - return 0; > - > - mm = get_task_mm(tsk); > - if (!mm) { > - *(char *)buf = '\0'; > - return -EFAULT; > - } > - > - ret = __copy_remote_vm_str(mm, addr, buf, len, gup_flags); > - > - mmput(mm); > - > - return ret; > -} > -EXPORT_SYMBOL_GPL(copy_remote_vm_str); > #endif /* CONFIG_BPF_SYSCALL */ > > /* > diff --git a/mm/nommu.c b/mm/nommu.c > index 498e01ee40b0..98596e60311f 100644 > --- a/mm/nommu.c > +++ b/mm/nommu.c > @@ -1746,8 +1746,8 @@ EXPORT_SYMBOL_GPL(access_process_vm); > * Copy a string from another process's address space as given in mm. > * If there is any error return -EFAULT. > */ > -static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > - void *buf, int len) > +int __copy_remote_mm_str(struct mm_struct *mm, unsigned long addr, > + void *buf, int len, unsigned int gup_flags) > { > unsigned long addr_end; > struct vm_area_struct *vma; > @@ -1781,43 +1781,6 @@ static int __copy_remote_vm_str(struct mm_struct *mm, unsigned long addr, > mmap_read_unlock(mm); > return ret; > } > - > -/** > - * copy_remote_vm_str - copy a string from another process's address space. > - * @tsk: the task of the target address space > - * @addr: start address to read from > - * @buf: destination buffer > - * @len: number of bytes to copy > - * @gup_flags: flags modifying lookup behaviour (unused) > - * > - * The caller must hold a reference on @mm. > - * > - * Return: number of bytes copied from @addr (source) to @buf (destination); > - * not including the trailing NUL. Always guaranteed to leave NUL-terminated > - * buffer. On any error, return -EFAULT. > - */ > -int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr, > - void *buf, int len, unsigned int gup_flags) > -{ > - struct mm_struct *mm; > - int ret; > - > - if (unlikely(len == 0)) > - return 0; > - > - mm = get_task_mm(tsk); > - if (!mm) { > - *(char *)buf = '\0'; > - return -EFAULT; > - } > - > - ret = __copy_remote_vm_str(mm, addr, buf, len); > - > - mmput(mm); > - > - return ret; > -} > -EXPORT_SYMBOL_GPL(copy_remote_vm_str); > #endif /* CONFIG_BPF_SYSCALL */ > > /** > diff --git a/mm/util.c b/mm/util.c > index bf0513d1d3d0..2eca27b02791 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -1061,6 +1061,68 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen) > return res; > } > > +#ifdef CONFIG_BPF_SYSCALL > +/** > + * copy_remote_mm_str - copy a string from a remote address space. > + * @mm: the remote address space > + * @addr: start address to read from > + * @buf: destination buffer > + * @len: number of bytes to copy > + * @gup_flags: flags modifying lookup behaviour > + * > + * The caller must hold a reference on @mm. > + * > + * Return: number of bytes copied from @addr (source) to @buf (destination), > + * not including the trailing NUL. If @len is zero, return 0 without accessing > + * @buf. Otherwise, @buf is always NUL-terminated. On any error, return > + * -EFAULT. > + */ > +int copy_remote_mm_str(struct mm_struct *mm, unsigned long addr, > + void *buf, int len, unsigned int gup_flags) > +{ > + if (unlikely(len == 0)) > + return 0; > + > + return __copy_remote_mm_str(mm, addr, buf, len, gup_flags); > +} > + > +/** > + * copy_remote_vm_str - copy a string from another process's address space. > + * @tsk: the task of the target address space > + * @addr: start address to read from > + * @buf: destination buffer > + * @len: number of bytes to copy > + * @gup_flags: flags modifying lookup behaviour > + * > + * Return: number of bytes copied from @addr (source) to @buf (destination), > + * not including the trailing NUL. If @len is zero, return 0 without accessing > + * @buf. Otherwise, @buf is always NUL-terminated. On any error, return > + * -EFAULT. > + */ > +int copy_remote_vm_str(struct task_struct *tsk, unsigned long addr, > + void *buf, int len, unsigned int gup_flags) > +{ > + struct mm_struct *mm; > + int ret; > + > + if (unlikely(len == 0)) > + return 0; > + > + mm = get_task_mm(tsk); > + if (!mm) { > + *(char *)buf = '\0'; > + return -EFAULT; > + } > + > + ret = __copy_remote_mm_str(mm, addr, buf, len, gup_flags); > + > + mmput(mm); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(copy_remote_vm_str); > +#endif /* CONFIG_BPF_SYSCALL */ > + > int __weak memcmp_pages(struct page *page1, struct page *page2) > { > char *addr1, *addr2; > -- > 2.55.0 > -- Cheers, Lorenzo