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 32FBAC021AA for ; Thu, 20 Feb 2025 01:02:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AA4CD4401B3; Wed, 19 Feb 2025 20:02:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A54C74401AE; Wed, 19 Feb 2025 20:02:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 943594401B3; Wed, 19 Feb 2025 20:02:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 76ECE4401AE for ; Wed, 19 Feb 2025 20:02:50 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 16C4A4C3DF for ; Thu, 20 Feb 2025 01:02:50 +0000 (UTC) X-FDA: 83138523300.06.5180950 Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by imf24.hostedemail.com (Postfix) with ESMTP id 257C118000C for ; Thu, 20 Feb 2025 01:02:47 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YPcAAeHi; spf=pass (imf24.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740013368; 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=QVTcMOsra+Zlj/NG7dWllagtUZ69SiTounS3LmJQVIo=; b=oO9OcFA5srVhBKf+6022UES7xlpCtgcpm1xieMcGqhVcQpsAiidCKef78NpntHoM5wt7wv mBf6N62pTS+ykY08aeeox4fLvR7AYnJYb2UxE5hNzNPQWFfW6Kt7/l0qgfx/SHWa8XN/Vk TD9l9VqqC2fPtGmcwOt7HZbhCfRbfV0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740013368; a=rsa-sha256; cv=none; b=sLCiTrXe9a20YizwnVQ7Ds9+S2YqFv7IAD6VLZOJVf0Jm0bhQrsR3bHG4/VILyiCdXkG65 pbbPTDjkVdKHt6mzGT5q37GM0V0q3TsPFpjJ0edZ8CqT0DeDjdIPNacsJUydOgFOqSL8/o jkpMErmc1a4iWDjRZmEDAdmARzS14Us= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YPcAAeHi; spf=pass (imf24.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.216.52 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pj1-f52.google.com with SMTP id 98e67ed59e1d1-2fbf5c2f72dso638593a91.1 for ; Wed, 19 Feb 2025 17:02:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1740013367; x=1740618167; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=QVTcMOsra+Zlj/NG7dWllagtUZ69SiTounS3LmJQVIo=; b=YPcAAeHi0Si+a7sM7WhdFOzCNVn2YSucHM4hlih/I+VkoUCXOgI6OUQPHQgNtkGoqB gPJ/S/BjIH6GH/tfpyO4QasRHTUTm9ns7UP6fse+C23TwWRyH2xlXnYG4PMBtpp/axAn egmnunSiJLV8DNDtDCd2Xd4kGmKLbkz/yJFM7IaO16ouVvsywtriVb1fV624KkwNJU/K PnrTwMkCbXmTifh+IQ5ap1byFIL3Kc2b2+ddjNBVhMbqfdLTNacPKO/5BqZpuyhpoFvi A3hDRMSNWxowBBTSt8EWVH77zYE3hFDuuy6bgLk6Z5ZNRKhxnIz72hkEEHWGKRmBGnOK cwxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1740013367; x=1740618167; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QVTcMOsra+Zlj/NG7dWllagtUZ69SiTounS3LmJQVIo=; b=h1PDgaYzxzu/K+nKWdS5EOKLEB03Cj43R7CZIjn/ad4JgOw+P1MaBkh3w1dix6RQ6e N30vB/xLbx24RfDPqEW+LVyZ4sh/MXCBcKB6zkryDLuaTFo3uTOwSo/b6oRlKUX7iNve vPiBNvMOUDSyLpB+G1uurGlVvJGimpDva88pMAEi0torkzKsPJCnaR0FB9H7vdTcn9e5 qFzgI9Iqvmhymx9TEiL99+mqdvU1fcH7IUUK+mEMj/5JSE0m9osqI/Z3Apfl5UUCwLCW hqhrMTqf5rGxfFaBG4HhLU0m1IGrQoB0gdJNDeZsbu/yxAslKQWNBKOJLgPMjlTLvTrv k3RA== X-Forwarded-Encrypted: i=1; AJvYcCXN29KmWQO7gkjC9divXnNydSVxG0hxHSr40ii5zcD61ATh9xp0jaU94cY6iOvV2xf5clJ+TE2QTA==@kvack.org X-Gm-Message-State: AOJu0Yyb1zgE0NbzcJSAXSINFtX14IoVn+DM6PQe92iozgHJRwG7uiRv CObOLFcX6vOnFy1zjTJUW+JSTHWuB32qUXZYSJlGqSRi6Ni4Qe+11FofDiA+4+kSU7zcVacQ/Lv F1xPs1TVYI8aZH49ggmIxQqZW82c= X-Gm-Gg: ASbGnctCe/ALjhJHu5Xs0dEXhBFfOZhfEtwT3YlWpIOmPU+eiacnBnoAyFBo7ee78wR qGp36xN5/3GPGMvnQ0FHOCzgr2mLPa57CkjPxz/md+jYm9zw2U2HySwe46mma5RmzfK2ODlTkan cds8DaqtcP0+a2 X-Google-Smtp-Source: AGHT+IFqcbKwoplFU/mD7j1kk+bDxgBsJghwRlT7khsWXCOjWNsO9utyBNm7gxLGUJtVgN+dObdECRi/pIgCJ96d6V8= X-Received: by 2002:a17:90b:33d2:b0:2fc:2a9c:21de with SMTP id 98e67ed59e1d1-2fcb5abe20dmr9040184a91.35.1740013366909; Wed, 19 Feb 2025 17:02:46 -0800 (PST) MIME-Version: 1.0 References: <20250213152125.1837400-1-linux@jordanrome.com> <20250219161821.6f05272f1a3131ddfe978865@linux-foundation.org> In-Reply-To: <20250219161821.6f05272f1a3131ddfe978865@linux-foundation.org> From: Andrii Nakryiko Date: Wed, 19 Feb 2025 17:02:35 -0800 X-Gm-Features: AWEUYZmGu7u0zYNUgfweDK5lveU6SluqZCoNdu4Xwgaq-JX0mEaOJ5M-NzYN9-8 Message-ID: Subject: Re: [bpf-next v8 1/3] mm: add copy_remote_vm_str To: Andrew Morton Cc: Jordan Rome , bpf@vger.kernel.org, linux-mm@kvack.org, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Kernel Team , Shakeel Butt , Alexander Potapenko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: 257C118000C X-Rspamd-Server: rspam07 X-Stat-Signature: 4ssrd15qswqufzocuskjnc8ayufp41ye X-HE-Tag: 1740013367-889786 X-HE-Meta: U2FsdGVkX198aAbWkDBUIhwaNIdAht8hWX9xT+Qo9QQPfNgo+tWZc69INFJZn9dTvFWoJ/LvEAUvfclzJn3CWp6LRwcsrMHrFUIpsyeLBGWzDj3wBHrKE11rWxfaFRE2UT1jVqr4PQnmTmUBJag2WBBJ0k28hTkx07U1dBvwJe1IS784698nzXv7NhPj/anRSetjXvyfVzOcdGJoTYjZkTGYU0dNkksHRXty9iiGD0clpJRMl5i2nk3o6kdqJI2uHhamOEbCFYJwP2xQJSTwYqoS/hEAr6sVfnUi8zEcGF7+0vge6cQ7Me1oYtLo5ALLNlKF0VykdXxsv+Lkh3yzDQ/V7WlZJ2quVYezpL3W+ohIquQAs1pYmm/oUymuQ2N5MPeyioy++yXmaZCa1rKPNK7V8+joh4fBvP45ggjQ7KrCdujA6igXzimSXQDlx4JFepcfCZ0GVyNvHVbSt3+J5YKV7S3wQq/QQWRDcycyrK/vkzpJeckGEtj+tNeR+g3m0ttQJs5m3EVsNyruSlhVOe2WRWtBSyxSwjF/cAQN5clk1Famc29NQWKVxspb+pWEsD+Zo+uY98IO2/lWBrFghFWdYiaDMrnlKP7BHBM91YrWAGacjkOerYWvKbQ3a7QBH+C5Sz08kckqpYcOklURiYupZ1xFzuGLll5q+qioNqEVHHT/1i/DDbl3bnGWItuiQgfKY4EINCQAtBv+Nxy8A9VcktuK1j4A6uNVAZRZsWIaCROoTPZTuo3oWrswPzDDQ0Mn2sx8dGqPh2DXW9RTFkz07hxI/xXLepsF8HKnZoBc/w7mT109bDzY+z1dZf5BmfqujhC5EQ2eKIakpCaUXnaGqwk09UkLHnCgV2lJUHodutzGKxOQ5J1at+9DBMWYuSjsmW7TqkpVgtBwMPQjfhtEWPl5TfnBggXf2EOR6zDFh9q9GNdjsIkMsFPct7SJCDv6HClkPVx2bm44Yem USiifoAc 4hdvya07f1prfQUh02dgQCjiCXoq/WmnCroI9jr20UuL2/an9ArgpDVyHOjwPstGMFyZfK09T0d1GM8nocHNTdKQzArAcEPM+S7ny7rt3GE7vJVNrYkzRDvyF74kTlTV+uZdqNcYOZIL6SSGUMsCnB/3d+1KXvikvUcpciItetpUAXmxZiOGcHOFUSyLRjDIycRUXRhzA1YEqZlYpGN3Qcr/O07rJsTFR69nFBlpHJksvwOb+zZEZ2BhYi7C7PhYjHIPsSLwIgzz3SHDxTWwKjN4qXrsfe7C6aDSAFvLdVPWBwlykR4bdBjUtN5CagXClADGOBrNS49aLdDU53G+LPtay4Ce/SGX8w++JIW1K6WSpV0WIeNxB38BP6NMdMikuNtcqKoo7dlSE6wzjDA5Yp4ASsVIIgL1PLjepkTV2st6VNijZ1rIPdQv2w/uzMBiri+Ldu+kdjDJanZP0bSEYMUwA9VD7ncueW53C0+6oFLPCd2wvAeUsT+0W8AQjpuP4pcOj/S/LDU2RhWE= 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 Wed, Feb 19, 2025 at 4:18=E2=80=AFPM Andrew Morton wrote: > > On Thu, 13 Feb 2025 07:21:23 -0800 Jordan Rome wro= te: > > > 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. > yeah, it's a straightforward #ifdef CONFIG_BPF_SYSCALL guard, I'll add it while applying > > ... > > > > +/* > > + * 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 ad= dr, > > + void *buf, int len, unsigned int gup_flags) > > +{ > > + void *old_buf =3D buf; > > + int err =3D 0; > > + > > + *(char *)buf =3D '\0'; > > + > > + if (mmap_read_lock_killable(mm)) > > + return -EFAULT; > > + > > + /* Untag the address before looking up the VMA */ > > + addr =3D 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. :) dropped the comment, but yeah, it's coming from __access_remote_vm(), of course. Seems other users of untagged_addr_remote() don't bother leaving comment, so dropping the comment seems appropriate (and anyone can actually read more expanded comment in include/linux/uaccess.h, if curious) > > > +/** > > + * copy_remote_vm_str - copy a string from another process's address s= pace. > > + * @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 (destina= tion); > > + * not including the trailing NUL. Always guaranteed to leave NUL-term= inated > > + * 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? so this was not meant to catch negative len (that's assumed invalid API usage), it was more about handling len =3D=3D 0 case, which is legal for access_remote_vm(). So I fixed it up to `if (unlikely(len =3D=3D 0)) return 0;` explicitly to keep behavior consistent with access_remote_vm(). > > > + mm =3D get_task_mm(tsk); > > + if (!mm) { > > + *(char *)buf =3D '\0'; > > + return -EFAULT; > > + } > > + > > + ret =3D __copy_remote_vm_str(mm, addr, buf, len, gup_flags); > > + > > + mmput(mm); > > + > > + return ret; > > +} >