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 7253CC021AA for ; Thu, 20 Feb 2025 00:18:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A33EA4401AF; Wed, 19 Feb 2025 19:18:26 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E1EE4401AE; Wed, 19 Feb 2025 19:18:26 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8AC594401AF; Wed, 19 Feb 2025 19:18:26 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 678714401AE for ; Wed, 19 Feb 2025 19:18:26 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id E6DE5AE6EC for ; Thu, 20 Feb 2025 00:18:25 +0000 (UTC) X-FDA: 83138411370.23.BDAF791 Received: from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254]) by imf01.hostedemail.com (Postfix) with ESMTP id 3933E4000F for ; Thu, 20 Feb 2025 00:18:24 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="yWq/uhKW"; spf=pass (imf01.hostedemail.com: domain of akpm@linux-foundation.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740010704; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/0YqJQ6bDaENx/D7r8uuJQEzFUe9y3kylgp+m4XDz54=; b=1n8LsMhcc6EV05zGhwOCmWkBtclVyAnLvHs5cTKrC+cmMHnW7m3gCjRbQBBClwggtyz2DK r5r4JiIMT2hJRoppoPy29ag97vipycBCpDij3e0Sn846t1CDlFM0hghENEwQkcg+mkmUau stAkpTrSSmuoh//PGevha5Gtzc93MuU= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b="yWq/uhKW"; spf=pass (imf01.hostedemail.com: domain of akpm@linux-foundation.org designates 172.105.4.254 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740010704; a=rsa-sha256; cv=none; b=HWBQf+iYApU0eqzfMI8Gcj15Tm4KjIAuR+elLW2znGamk+fZ50LEV/xcI3MpLUb4KmdIdX 0DdCDy3UXFDHDVW8QFF2wCvj0O2Ubvw33ue2JrPDcDhGDw3sOQU7xZ6nNdqAxloE9MhQwQ jH/a9Zrj17jRBX9lmj4inwm4qZs+Po0= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id EE9C0611F8; Thu, 20 Feb 2025 00:18:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 71AECC4CED1; Thu, 20 Feb 2025 00:18:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1740010702; bh=lczxXNo8KAGLAhmOJvMVTwpRkP9aRLwOp36UTrgSXVE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=yWq/uhKW5Z2fp1kGJijuBGSgTmlgqxvUB+2LO9ho2+Wm6TRXIwkZVhCkqkmRGO2te jvbXDCtHjcraAoRyZsRGF6XQR7UxtI3jAeHP1xXzTf1hTy/+fifvl7mwdmkz/GaLEL +llj+KL9dcVdXSzfw4pMify7AM/AbykTdjW3sYao= Date: Wed, 19 Feb 2025 16:18:21 -0800 From: Andrew Morton To: Jordan Rome Cc: bpf@vger.kernel.org, linux-mm@kvack.org, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Kernel Team , Shakeel Butt , Alexander Potapenko Subject: Re: [bpf-next v8 1/3] mm: add copy_remote_vm_str Message-Id: <20250219161821.6f05272f1a3131ddfe978865@linux-foundation.org> In-Reply-To: <20250213152125.1837400-1-linux@jordanrome.com> References: <20250213152125.1837400-1-linux@jordanrome.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 3933E4000F X-Stat-Signature: eduso4o4dz1pcpn4wddj38u5pkjkasbm X-HE-Tag: 1740010704-985917 X-HE-Meta: U2FsdGVkX18/dj8oVRy7zpncCs5UChCabaZCV1sCjg0F5ib4HPzUJvAbissEygpjHdW3qz5V0b+35g3vo9CIppR/yfRljE0yd0ROngSDTnGG7WPllv+/CPX/JAiHnnHYybamUUwUUax3xpKACjqyqtGTnb3WDObtzAxe/zlnSwaq1vtp0LG7iFpBooPXq4zvwApaz6RZGi/4bVy9H0dj/OKF8yVxUshDgo3hzsXlVNqZSopTaMkKC62iXFy3WG7S8f3W7XfDW/w9wGA+vBdLe+nmJOCipsObwb+WDwxF3k+zmtMeMun2bMXhb8s9HcpnVT5De6sVtOfGtOHC4CUSYrWAOgpyl3TjRrS4Gc/rREQyTcbILB3/9X/5MvuZA8k65YicL/rD2/067jlpVVubqiqC9RFWQDcbutJ8KxBeabGmFR8jWjnE0f99hK3BXZ6veSYzxqBMEu+R8qA4KIrm88SM1dN3CQ2CKLNl4HEdTgUu/0AXOOw/PfqtNXNVuVaXnfr1uYheyT4ScW+jDAcfoeS/qqCJeqaa3EzWarQd16z/4K8k9AA+FRnFt4dhaZWQs7B6KjgzgY0/MT/pG6IiBGgpUB4hNZSQbeT8qVPzD0PUoTLDp3NQq7oHCGWrttKZKO7+5A+B7OKS8yLx193NNuJFNaIMiZ+V1BQVSZYFdfLspqJiCGpgIqIvKBsuGEk8yiUuXgbZxZL3NdU6TCXMeVkvWZfjJVa2qFHpu2r7fxthG7LTvbr6csZzbgx/6yzSc4j7193z00I1lQIQdFz6voh46mUiRj0Lb1yBEfdTEdtFeZ4SlbFavwytRPVm/+WQEkcFxOqd8ziLTgl74s+mkN6+M9a2jJjehsmZYJVUmDBkCbFGUEx17+8QF65s+zAZK+L1CZ6YAk2IlpvilFZsBfHDF89HbhFtj/KhMYGyTC644i6ufVhY3+Y70xTJbQMEfC7spFmUtV1YVaa/ET5 jik4fqEg lUVo1jtGDc9VjBEI2GX5XbdHriBf6I20U4yn13kxS6fEtPcU2k2F1XVQg5Oj+YYzn3CSItyJeAAKYHtsDiFz8+M/dTJK74o+G1jidre/wTF+a4DJ5Uj5zg9X7ccMA2D/N6HC7B++yJJBZSTl73YlFkxF/24M7+YZ0W2iKu6pSHOj0dv3CbmCP2EldURRZeOJIFDDf3ZvStewY/tgVk8qjpOaoZq1eolOTo2TImgB94LG3FdXPuJ/+9KWG8I+S3NN9o4pcKyUO+qt2ubPAvhcjwp/NU09LnH/gzqPtks5ApkQ2Gq0pInHMmM5ZM3WXgOLg9hHEHp/niPHRJ7LdM5bg9dfPhAn6VTYP6GrL 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: On Thu, 13 Feb 2025 07:21:23 -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. > > ... > > include/linux/mm.h | 3 ++ > mm/memory.c | 122 +++++++++++++++++++++++++++++++++++++++++++++ > mm/nommu.c | 76 ++++++++++++++++++++++++++++ Is there any way in which we can avoid adding all this to vmlinux if it's unneeded? Any such ifdeffery would of course need removal or alteration if callers other than BPF emerge. > ... > > +/* > + * 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); Well that's a crappy little comment which you copied-n-pasted. It tells us "what" (which is utterly obvious) but not "why". whodidthat. > +/** > + * 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; I wonder if this can ever happen. And if it does, should it WARN? And returen -Efoo? > + 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; > +}