From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9E19C021AA for ; Wed, 19 Feb 2025 23:33:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1D3F944018D; Wed, 19 Feb 2025 18:33:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 15E79280276; Wed, 19 Feb 2025 18:33:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF1AB44018D; Wed, 19 Feb 2025 18:33:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id CC19F280276 for ; Wed, 19 Feb 2025 18:33:20 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 3EF7DA0EFE for ; Wed, 19 Feb 2025 23:33:20 +0000 (UTC) X-FDA: 83138297760.10.BEB0047 Received: from out-173.mta1.migadu.com (out-173.mta1.migadu.com [95.215.58.173]) by imf01.hostedemail.com (Postfix) with ESMTP id 530924000C for ; Wed, 19 Feb 2025 23:33:18 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=wZOEJqhN; spf=pass (imf01.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.173 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740007998; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=jht595c86MqUyIgPf62UjPPfZmeUGCfiZOP9eYz5J+Q=; b=VYI5DPhc7SuW+BdZzUlLRWXRl5wwDBJVywl6DHbzB4sz0M2LMzQgvl6I93EZgf/+esHNGi 8sRzEyTcxju3jAX2tkgLljTIkbXpsEAYSgDL5SsQappOzcUlt7eNYEGKoBnD1E/yDIgy3D Y3RcvmeYqM7mD9dFGWWE3NpJl50Lg74= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=wZOEJqhN; spf=pass (imf01.hostedemail.com: domain of shakeel.butt@linux.dev designates 95.215.58.173 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740007998; a=rsa-sha256; cv=none; b=xCbuDVaq0TAl+Pnajhta7xeeQDREAQKM4mWSHBFs8ynbjChlKmMK5QWlPJn5e/P5COGu6r OIvwlHWmso50c2Yewt85GkxvL5aCIqrpf1xHybV++U4ZfUm4aNgRdxp5QZU8WtW7ydXsc1 KjFiM5PxtyrKy8BtWUhI81HTC3XjRtc= Date: Wed, 19 Feb 2025 15:33:12 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1740007996; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=jht595c86MqUyIgPf62UjPPfZmeUGCfiZOP9eYz5J+Q=; b=wZOEJqhNf9TUGhN44gExUpM3AHwFeMVC63OeNAePuED8ApSQ22ggG71E29ExUoZng74H5f CK20VDBv+W8UCsnhLMcOUVBy7bzUdm0ipTK8n1aieIRu1qp/wnij/x5eDwkmrCUEktPrvf r5AMmCje2XOohBPyxNwhL64i0PbsbPo= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Andrew Morton Cc: bpf@vger.kernel.org, linux-mm@kvack.org, Jordan Rome , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Kernel Team , Andrew Morton , Alexander Potapenko Subject: Re: [bpf-next v8 1/3] mm: add copy_remote_vm_str Message-ID: References: <20250213152125.1837400-1-linux@jordanrome.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250213152125.1837400-1-linux@jordanrome.com> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 530924000C X-Stat-Signature: r9d36x886wct1nuxoig6r1a1n7foyqbf X-HE-Tag: 1740007998-117721 X-HE-Meta: U2FsdGVkX19p/a8tvLh08uyy//H5hTrgIzI4ELaeyXG/mqfvxPYwizSzXcz1tfT6LmeRauggAFF1wp1zYr9PtgHqz/LDBv9YnKQ83otPnIKysTdclL44x+pUPstsaF+EyYPXxI3VzdOxs7vjcvEG5+RNfLVzJWoEuwTup+AcEtQvEMvPkGs8BWTmZsq9nGKR3I8EoPp+POIz359arZ5q0b8zaKkC31lt241csHTU0NtkkEphOu2otYjuzISfUf0+cR5a00+D8W6FEgK6RoJKgEa5/LKkhZALLAr57ygrLRNSGqmDNVlHuIjJOISsuOkvTI6jwR7irroviFlsuOMsoCR9ba/Q6lSL6SuHswB4Y/n6/M8zUIMblQlCxR7nyahfJNg97OpRaXiEEq+GnWNnXGs9JedklkaHfybK+7KC7G/dyP4eh9GhNF2BoDR4+M1P7PYcp6FNiP3A+fcCS4vm/mXCXAtQrPqIYr5xxFaCu7Xf8x7tL5D7kR/CIt4PzthzS/5NnwNco4gpjaEeyjpnjQuFxZ/FRuKciRi6c2t7BA3AqKieEjmOQetbMjF+Bp4XRau5LZq3yyx+9H5vWDjlYCpIy0z1rYJWvvrWwwVpaOcXf/WmDfmH/Y9ekd28uS6qBuI443aHm1KEgZyplK5/ZiKCs148L00g0EUGGSCROEfN2P49qK7HRW3ni3evqatDRPd5pdV96CN1gA21hXZXX19PTN2o2aQcpUQM4fGKwVFJf/IRcsYzYD8bwK9F5a6Gcb3KWEVDEd6FAzNF8yl1a2EbEf+GDzqMn4YdDYuMrJFMoUV0V2Y7rgPDw6R8Cb6uwAgSt1nI/GXBuh3sDSizftyKQGPnw0ZvWgbuNY4A20Npm9nqq/72mP708CGYcCyJXqUUogKpSp4dxOHb1eCVA6KgdMt+gl940W1Zqj04Y29YgBhdBBm03DAEDoF84QWVGSBnFpuq5hXIk8BJHY3 8DG/Cl26 CbfDxWYpPyMe3K7yDdHc+pSIr/VRaB0jQDihFtD+otIU5IOZ9Aq/qKnIrti2Og03LvJ2wXlhFLHS1wKKEZcIz+dt8EJ6vgTZwxvsvfPk8BytjjDj0LiUHXFWqGCMy15zagVI79OtVr/zvR2sL5swhZPfVk4idkwGFFdOhzcNFVJJlo5ACQF2J4d/uO29pHzIRCd2pR2bFE+YzAu1Xj6sxiJUkD59URmlVh7Xmcme+HUdVe9j7JNEOAsVW7JhVsKhepnUqpD7y87bkXrXCsew90PJJ5EkLUZcfcrJQ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Andrew, Do you prefer this patch series to go though mm-tree or routing these through bpf tree is fine with you? Shakeel On Thu, Feb 13, 2025 at 07:21:23AM -0800, Jordan Rome wrote: > Similar to `access_process_vm` but specific to strings. > Also chunks reads by page and utilizes `strscpy` > for handling null termination. > > The primary motivation for this change is to copy > strings from a non-current task/process in BPF. > There is already a helper `bpf_copy_from_user_task`, > which uses `access_process_vm` but one to handle > strings would be very helpful. > > Reviewed-by: Shakeel Butt > Signed-off-by: Jordan Rome > --- > include/linux/mm.h | 3 ++ > mm/memory.c | 122 +++++++++++++++++++++++++++++++++++++++++++++ > mm/nommu.c | 76 ++++++++++++++++++++++++++++ > 3 files changed, 201 insertions(+) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 7b1068ddcbb7..aee23d84ce01 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2486,6 +2486,9 @@ extern int access_process_vm(struct task_struct *tsk, unsigned long addr, > extern int access_remote_vm(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); > + > long get_user_pages_remote(struct mm_struct *mm, > unsigned long start, unsigned long nr_pages, > unsigned int gup_flags, struct page **pages, > diff --git a/mm/memory.c b/mm/memory.c > index 539c0f7c6d54..014fe35af071 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -6803,6 +6803,128 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, > } > 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) > +{ > + void *old_buf = buf; > + int err = 0; > + > + *(char *)buf = '\0'; > + > + if (mmap_read_lock_killable(mm)) > + return -EFAULT; > + > + /* Untag the address before looking up the VMA */ > + addr = untagged_addr_remote(mm, addr); > + > + /* Avoid triggering the temporary warning in __get_user_pages */ > + if (!vma_lookup(mm, addr)) { > + err = -EFAULT; > + goto out; > + } > + > + while (len) { > + int bytes, offset, retval; > + void *maddr; > + struct page *page; > + struct vm_area_struct *vma = NULL; > + > + page = get_user_page_vma_remote(mm, addr, gup_flags, &vma); > + > + if (IS_ERR(page)) { > + /* > + * Treat as a total failure for now until we decide how > + * to handle the CONFIG_HAVE_IOREMAP_PROT case and > + * stack expansion. > + */ > + *(char *)buf = '\0'; > + err = -EFAULT; > + goto out; > + } > + > + bytes = len; > + offset = addr & (PAGE_SIZE - 1); > + if (bytes > PAGE_SIZE - offset) > + bytes = PAGE_SIZE - offset; > + > + maddr = kmap_local_page(page); > + retval = strscpy(buf, maddr + offset, bytes); > + > + if (retval >= 0) { > + /* Found the end of the string */ > + buf += retval; > + unmap_and_put_page(page, maddr); > + break; > + } > + > + buf += bytes - 1; > + /* > + * Because strscpy always NUL terminates we need to > + * copy the last byte in the page if we are going to > + * load more pages > + */ > + if (bytes != len) { > + addr += bytes - 1; > + copy_from_user_page(vma, page, addr, buf, > + maddr + (PAGE_SIZE - 1), 1); > + > + buf += 1; > + addr += 1; > + } > + len -= bytes; > + > + unmap_and_put_page(page, maddr); > + } > + > +out: > + mmap_read_unlock(mm); > + if (err) > + 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 < 1)) > + 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); > + > /* > * Print the name of a VMA. > */ > diff --git a/mm/nommu.c b/mm/nommu.c > index baa79abdaf03..11d2341c634e 100644 > --- a/mm/nommu.c > +++ b/mm/nommu.c > @@ -1708,6 +1708,82 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in > } > 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 long addr_end; > + struct vm_area_struct *vma; > + int ret = -EFAULT; > + > + *(char *)buf = '\0'; > + > + if (mmap_read_lock_killable(mm)) > + return ret; > + > + /* the access must start within one of the target process's mappings */ > + vma = find_vma(mm, addr); > + if (!vma) > + goto out; > + > + if (check_add_overflow(addr, len, &addr_end)) > + goto out; > + /* don't overrun this mapping */ > + if (addr_end > vma->vm_end) > + len = vma->vm_end - addr; > + > + /* only read mappings where it is permitted */ > + if (vma->vm_flags & VM_MAYREAD) { > + ret = strscpy(buf, (char *)addr, len); > + if (ret < 0) > + ret = len - 1; > + } > + > +out: > + 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 < 1)) > + 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); > + > /** > * nommu_shrink_inode_mappings - Shrink the shared mappings on an inode > * @inode: The inode to check > -- > 2.43.5 >