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 4CC24C25B76 for ; Mon, 3 Jun 2024 11:35:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AF0E76B008A; Mon, 3 Jun 2024 07:35:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AA0996B008C; Mon, 3 Jun 2024 07:35:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 967C36B0092; Mon, 3 Jun 2024 07:35:47 -0400 (EDT) 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 7AC316B008A for ; Mon, 3 Jun 2024 07:35:47 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id B041F160C76 for ; Mon, 3 Jun 2024 11:35:46 +0000 (UTC) X-FDA: 82189372692.22.8603EB4 Received: from mail-qv1-f42.google.com (mail-qv1-f42.google.com [209.85.219.42]) by imf25.hostedemail.com (Postfix) with ESMTP id E97A9A000B for ; Mon, 3 Jun 2024 11:35:43 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ks69ldMm; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.42 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717414543; 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=2/pL0kZCeM1y16ZAOktPzW9q0OBMB22PikjjFd/rS+w=; b=JAi6fggwciDEJ1FWFjLunM+54+KSQ0/hbagc4PtotV0P70gXyz5YRE58l5oL7xtY+3yMvW 6baiyR6PJXJmommKPtyfkkj/R5x073lvTiSXjC7r9siJky5ZaFziaNXTzgxuEiOEyos1Ef 95cVh4DUmJgI6VbCeMh/3TlaHJ7a4JQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717414543; a=rsa-sha256; cv=none; b=ufl1PawG93WBfUu4B7jnvF2cVytez3Wlj00QKzJHLSiRhxb8+twvKpTkQ9IJj1P3ZIjTxa 0dveFfimUnlt8kJPsO4njeDk3OpYSpP2ktIPrxFMMQeYND5qR+/F1wrwNtQerg6u520dWF 18sdV2+so1vnSF968DKxODO9XgjJhag= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ks69ldMm; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.42 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com Received: by mail-qv1-f42.google.com with SMTP id 6a1803df08f44-6ae1b32752fso20545566d6.0 for ; Mon, 03 Jun 2024 04:35:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717414543; x=1718019343; 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=2/pL0kZCeM1y16ZAOktPzW9q0OBMB22PikjjFd/rS+w=; b=ks69ldMmD41rex58Z3rWTXOcTnIbm+Tsd5yiLJ3NIi1iR5TcQH+0aF7Ma22pVjvwR8 wbnx8sukZfDBopM8xZcRTAnBUJqlKbtCyJRwcn1EapUXl13a/XAZuw8jjnoavYHbIehV JnejKN1yMUY2mCozWtaZSVSjhwf0Il/cXmxdueC52EAiHHZ2N24u52XL/agmJgTsD3sX SgCeDxXLzjxcc9uERjaSw+VamoSrRV1l3Pv/jMe8UrNMHp2bfP+EWIn21k8Qq+9rPwyv SVJXwXzM6yW1MGqsWjhpDtAfYyFik6NURHfHli4CgHUm5G+cuRCrB+LWjcNoyLSYxxL0 ryzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717414543; x=1718019343; 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=2/pL0kZCeM1y16ZAOktPzW9q0OBMB22PikjjFd/rS+w=; b=lnQCVka9K4G32WJw5zDPP/U91JJE/rr5SaoZ9Tl3zJuwdL7KA+vHIBTPCGklTreWJT SQccwn9c3Pf7MkaLS4ReXsp4NJweYbyUlMJ/7QVe5DpspmAwNWH5/fW84tA9NwLPi+7g tZ8lCraOVEcRU8yepsDDmtFyajQn0J3wkV+9v0XnA8tr/8Fwl/4EERXEJHvPMLn3UZWK YtFg2UNqCOjdnuVd4ejEvdnPPBxnvoCIRWXAEEBNMG7dzaeaN+rP9h3MuXod3TELrTOw kxqp7JDneOaKvNGEiDSm7xxgRjSTSzvJAvc8r4eNHHwDpN3Cez4l/wctVq1dZ2xLDm28 4VcA== X-Forwarded-Encrypted: i=1; AJvYcCUNqen5KxGNfhYvOsm4OD0j5AR7fTWrGLnMtgsDkeDvcK/6yHLwYCGHFJ3RTjN/fGNGV1uGW0w7xTdSNBZTaw98iUo= X-Gm-Message-State: AOJu0YxeOwlzT23mxNJjh5VGxQiEDpaxt+XwFEONDAXY2/9TMY1bSdBb +927srZkNbRFQjRu4+SHuKiTr6PqkP9DKz1TTn0PUZ2OGYJBpnosYdUGgLTcwe1GvdUvn4m61F2 crczJ/HD8+3YNT7j7tZcP6JViTr0= X-Google-Smtp-Source: AGHT+IEMPGXe1xq7rGBpGwfaIRAsELArfyOIl90qn3qIm6Yayy3B/d/0+DEKGHoLO4/5D6X82RkZyCrmvpduHJpXVSc= X-Received: by 2002:a05:6214:3d99:b0:6af:c64c:d1a0 with SMTP id 6a1803df08f44-6afc64cd5b7mr18050666d6.56.1717414542934; Mon, 03 Jun 2024 04:35:42 -0700 (PDT) MIME-Version: 1.0 References: <20240602023754.25443-1-laoar.shao@gmail.com> <20240602023754.25443-2-laoar.shao@gmail.com> <87ikysdmsi.fsf@email.froward.int.ebiederm.org> <874jabdygo.fsf@email.froward.int.ebiederm.org> In-Reply-To: From: Yafang Shao Date: Mon, 3 Jun 2024 19:35:04 +0800 Message-ID: Subject: Re: [PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm() To: Alexei Starovoitov Cc: "Eric W. Biederman" , Linus Torvalds , linux-mm , Linux-Fsdevel , linux-trace-kernel , audit@vger.kernel.org, LSM List , selinux@vger.kernel.org, bpf , Alexander Viro , Christian Brauner , Jan Kara , Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: di6qj4e5yjg758gwma84e3pdymz8pw7c X-Rspamd-Queue-Id: E97A9A000B X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1717414543-980651 X-HE-Meta: U2FsdGVkX1/ZKHu9zK8ftMqz9VUmNv1GTV30nkQdiG6lkiyoUn8CpdBdtU3yJIdzHZGfpSNaleUNcjgbk/8SyoVYauZJjMXhYOhi2SsSuK7ojgzxOa7audnK0XL0jEJjxCpuWj2KlTyyoeJTBpxxbvvtnOQKZEj0yoCY65hVH8KgBiw3IauNM+fRyn17Ek2iX/0zYEpWZr/yuf7GOaMmFS2g4AWnTSasSWTNJwVjRFH1Jl+FFOUNfBmaekxyVuMjjfeCS5PMHUYh4TtbsERO/IEKnfsV/hr3PQhpBFVo/omiYLnui65hpc6g5fFdkj2sCrqmBYDIGQWZGPgpMwDqo7Qdv4IRmIT+EKVdih2XUHbGVE2uhMEALVLPLpG2ybFLf0XJXkBqGp4k1umbnZY+j0/NJUMkgW7yETcuMRx0kRcbZrufYsfCLEV4ofiB0Nv4BCEl3pXvgcCmxM5WdslYGWHixNFIwz+PELBwQ4gLkZ7tpvySvTD7rHJQZHJlMOrE4cLka+EhckcO0oCOW0TaRHp9d3i66cn2riKzHDohEFlBMgHv3M9TQkw0x0sG5CQy/aQCDV49jm146hl1q3W54fd3+x1gWV8eYasISLsATf25VuyC3Q3C65uEcP5/kVCD2Ri9mNGYPA21FeGIq5x7QtWi91A5xguseZntE0m6EboYVYNK83+/Y5jz5zIh8NBXTIK0GP1CJxDPq3kw5h+TkZHsfTSIsrlSZ8fvHKOGqUOsJB42mUjnAYi8W2rYMvJ/2iIa8oa4Pvo/oRpBFi1DylcggIkQAdPtPIxP076A5cjtemilZdM50Ysn7qEqU6sHZbOjH6y2Uj+A7g9aRppQGWSIKQSaapG3coetDdBx0HcjZS+NvS5LogdvRnTR7SH/s5km3L4RwZSAHHC+WBQQoC5HjnHfAHtnobJMgRGgJl6cy7MpLkwrIW+ZRkfHnfOhK2/CsLjgOVzyvXGIH6F tYVvGKDp MeIrNxJXNrrNObEmTPe706Ctme3CeD+ALM2gbq+E84casngq3YOO1OOJt71Lh6NTPWXcnUW/aCljhYsjfQQyf+Bu1jrRkJXOI3znlG069uYPW4ekgvaHT+hNjlWyJaIg1M2DEIEBR0FCd5KkqpxySSIjXaJon/WcCH7TUlyNVKrxG6FgKBsXa/PjP8FJU97R568g+2XljHl/WzQuuSj16POjSL4fy/DCopUggMUvsiumXv5ugcN8UIfA17Bl/6PRBS92UYDBSlsOzTtfa4Eqw3sZfk5C89IjFPsf5jELr+t0yl/3DLcqlYOvgZPgXuPAygCoBzL1jCy9lrqVqNKLQvDrJAeCB7a0agBujsgKtItTApYB9lPijTKXBCzcogllCaTRmcLGCxYmrHOPA8EYfwc31ybNdYqwnQjGED/U6wl9rv6dvVZwagEe4LVRvRoL+sHNHTunENfbA1ITR091BNyNruiTjROtBNI41ELaQOAt5hGqFXWp4qjEWHUa82BqRIUdcpljEMguK3f9QM3wji+uFaigpM3fxynEf6Fl0e/bf0ny8Z0cDT+V9LBcdHD0U5BFesf9+uHiLQ4o= 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 Mon, Jun 3, 2024 at 2:23=E2=80=AFAM Alexei Starovoitov wrote: > > On Sun, Jun 2, 2024 at 10:53=E2=80=AFAM Eric W. Biederman wrote: > > > > Alexei Starovoitov writes: > > > > > On Sat, Jun 1, 2024 at 11:57=E2=80=AFPM Yafang Shao wrote: > > >> > > >> On Sun, Jun 2, 2024 at 11:52=E2=80=AFAM Eric W. Biederman wrote: > > >> > > > >> > Yafang Shao writes: > > >> > > > >> > > Quoted from Linus [0]: > > >> > > > > >> > > Since user space can randomly change their names anyway, using= locking > > >> > > was always wrong for readers (for writers it probably does mak= e sense > > >> > > to have some lock - although practically speaking nobody cares= there > > >> > > either, but at least for a writer some kind of race could have > > >> > > long-term mixed results > > >> > > > >> > Ugh. > > >> > Ick. > > >> > > > >> > This code is buggy. > > >> > > > >> > I won't argue that Linus is wrong, about removing the > > >> > task_lock. > > >> > > > >> > Unfortunately strscpy_pad does not work properly with the > > >> > task_lock removed, and buf_size larger that TASK_COMM_LEN. > > >> > There is a race that will allow reading past the end > > >> > of tsk->comm, if we read while tsk->common is being > > >> > updated. > > >> > > >> It appears so. Thanks for pointing it out. Additionally, other code, > > >> such as the BPF helper bpf_get_current_comm(), also uses strscpy_pad= () > > >> directly without the task_lock. It seems we should change that as > > >> well. > > > > > > Hmm. What race do you see? > > > If lock is removed from __get_task_comm() it probably can be removed = from > > > __set_task_comm() as well. > > > And both are calling strscpy_pad to write and read comm. > > > So I don't see how it would read past sizeof(comm), > > > because 'buf' passed into __set_task_comm is NUL-terminated. > > > So the concurrent read will find it. > > > > The read may race with a write that is changing the location > > of '\0'. Especially if the new value is shorter than > > the old value. > > so ? > strscpy_pad in __[gs]et_task_comm will read/write either long > or byte at a time. > Assume 64 bit and, say, we had comm where 2nd u64 had NUL. > Now two cpus are racing. One is writing shorter comm. > Another is reading. > The latter can read 1st u64 without NUL and will proceed > to read 2nd u64. Either it will read the old u64 with NUL in it > or it will read all zeros in 2nd u64 or some zeros in 2nd u64 > depending on how the compiler generated memset(.., 0, ..) > as part of strscpy_pad(). > _pad() part is critical here. > If it was just strscpy() then there would indeed be a chance > of reading both u64-s and not finding NUL in any of them. > > > If you are performing lockless reads and depending upon a '\0' > > terminator without limiting yourself to the size of the buffer > > there needs to be a big fat comment as to how in the world > > you are guaranteed that a '\0' inside the buffer will always > > be found. > > I think Yafang can certainly add such a comment next to > __[gs]et_task_comm. > > I prefer to avoid open coding memcpy + mmemset when strscpy_pad works. Thanks for your explanation. I will add a comment for it in the next version. --=20 Regards Yafang