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 81FBCC25B74 for ; Sun, 2 Jun 2024 18:23:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EAEA46B00B9; Sun, 2 Jun 2024 14:23:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E5E666B00BB; Sun, 2 Jun 2024 14:23:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CFE436B00BA; Sun, 2 Jun 2024 14:23:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id B2B546B00B9 for ; Sun, 2 Jun 2024 14:23:32 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 617F91A0203 for ; Sun, 2 Jun 2024 18:23:32 +0000 (UTC) X-FDA: 82186771464.25.6E07393 Received: from mail-wm1-f46.google.com (mail-wm1-f46.google.com [209.85.128.46]) by imf13.hostedemail.com (Postfix) with ESMTP id 8654520007 for ; Sun, 2 Jun 2024 18:23:30 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ff9VCpZK; spf=pass (imf13.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.128.46 as permitted sender) smtp.mailfrom=alexei.starovoitov@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=1717352610; 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=Gr/6Wsm0egkRb/ylviGUiuT2dKJTjbzFzZRbk1jvK28=; b=ksdVb/u4KQ8HsY6p2TSGQxqIKhE6sXaE1yzSymPzU52jCl5tLuXsIMIXzxeEqu6XhPuvSV gv8JAO8bV9Iuvhy0f+wK9BG1UT+9yeKK6lXyC2XSSam9vU7YEfoVX8fHzsGeBxTjesT+FY tuxUBL/4ULOaRaG+RY5MI1B0iwarjBo= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=ff9VCpZK; spf=pass (imf13.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.128.46 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717352610; a=rsa-sha256; cv=none; b=OgVIebRBH7WJRUR86URyE9yr6oVgYCUb5pC7VPpAGjxyfEgIYBW6g80FR3dK0ImZRWHRLC qVCOcDL9AQax/Jn5OZ+ghIYc5xvA8avvGKhcD81FcW0tK17xd0pPVGLcbcxbdU4eMxXyZt 6gsECmTTYnONiFZNpkbRKx3ZkLfrdcs= Received: by mail-wm1-f46.google.com with SMTP id 5b1f17b1804b1-421124a0b37so23518335e9.1 for ; Sun, 02 Jun 2024 11:23:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717352609; x=1717957409; 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=Gr/6Wsm0egkRb/ylviGUiuT2dKJTjbzFzZRbk1jvK28=; b=ff9VCpZKzbGj2RWjqciRgl4nzk4LfMkOVSQ+EY+H1LY6Cv3v+AU9lxzb0EgHwQ2rfM oFFKCHF8YWy9Vli+MFuEeVSkG6espjWZMEVV0/v9OsWNvfxJLUA1JlTL9thm8PajZwUe EYkRk5WkMdAVvOmbUHS2q1xtm8Bb0kQDeJ7LPkzSJznpnJ5cS4esM5JSZSEObFx5mr/t Ht35hnGkLeLzErk/ZIlfOvEprL0lDXjWZOjZ3hzHPC2ZkkZ0g+Jmie8WyoTnjTi1ZT9B 4ve8ceAVXJStTTxaZ9/TUvXGWd943nOEzcWmIFuNytzYHz+jIgTaFoUQ45fQq9eXWW93 orjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717352609; x=1717957409; 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=Gr/6Wsm0egkRb/ylviGUiuT2dKJTjbzFzZRbk1jvK28=; b=OHVqIbVOgTVzoRzaGY4zg6ZyzHjAU0sbH7QvY/89H7sGBXjyNgA/LGrVluSf7aRmFK 1ObEbEaleoBRnZL0hnS13OHYXBSPIrIDJPLcJSudDDQKDmnFaHWOGo77wIxv073sN96t DyHN2thbPpZjk69vGsbILmJFvbKoTaZ16bO70H3zYFV7jEVgxVs9TqVj/Eevmg2FRNGo U6ugblNy8vRp1c7QKLH0Rr8rMnOopQxC0+0PQG07ofQ4L2rzOGZzb/E3kSh8C9tzOfee tC/fnM2ycC3Au9zUegcxaAq0dN1vYCtFIAUFGuP+CgRcPvFrcgBbfsx8Aw5DWHX8Azw6 t20g== X-Forwarded-Encrypted: i=1; AJvYcCU1kTkgyHbBIrideCol8bNZ5HYpQi8UjqjKGAqRa1QAL6UYxBAtyjAVby5ECZ9xHmHTVcAh+lid20mw2LEKKHPjkLo= X-Gm-Message-State: AOJu0YzJ2RH+kvaY5YAONsSrRSkiPX3haDihevF3+PPNR6oCWpxYQ9kA K1QfHttUr/hv2EUrWSCQJA7fSRpnigYSrrioLTeYdXuxt0TnUHVEbkrGR77L3oLjVutS7XVj3QF n349x3PyiY05QZoVMv9ODKTNWYao= X-Google-Smtp-Source: AGHT+IEqBUebKrFkVe/BMrUTZcNp6QHc5RmcCuTRnWLHtkrfVemXSY7n9l4vdu8nv6VxYuVVes1X9ysaRl9XXhiuDnk= X-Received: by 2002:a05:600c:1c19:b0:421:2adb:dd4c with SMTP id 5b1f17b1804b1-4212e07009amr62269425e9.22.1717352608691; Sun, 02 Jun 2024 11:23:28 -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: <874jabdygo.fsf@email.froward.int.ebiederm.org> From: Alexei Starovoitov Date: Sun, 2 Jun 2024 11:23:17 -0700 Message-ID: Subject: Re: [PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm() To: "Eric W. Biederman" Cc: Yafang Shao , 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-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 8654520007 X-Stat-Signature: s8p98jh5grjfbcyarq8rab1g1e5rgi99 X-HE-Tag: 1717352610-283627 X-HE-Meta: U2FsdGVkX1/B/+vYSCSrHzvGoMFg2rnbrpIrAzJGxgidYigBigzzTfN4Uvj1jQcyv7NFg20TkS2a3yCd2Ua/XK2HK7qxr9s7jwTgDJ4F0YZHT42e/hc+dlEYM7vdAMeqolLbIvTjXWMmLNNBPCTu75nttdGTy4g7WRC08XoTqVLMUHlj3rK+a5lJvSzKJS3iY9kHOGfMPrEy53i/qEnHVxBuZVWGoXFZZ2Yn07yHI7x8Nt5G27RjPK08YCXAsX8S8ZwnkgqvSjXbSVDXJTPN8x0xrPNQr0iDl717ya/Y3GgzDAnzMmLvVbnXpNRtxrC4jVtn/raqp5cL5BtOZ4e49fuox3i6jhNsO6n0UvfctAtpRj2027j0n8yWoGpUbCCPmiR0KdNPFxvzABDdupeM9rQoslkP+DZXNCMRBhwhpFVU+v5Pc+uhSDihX7k9zZO8KF5XMpDVK5C6Jo1TRuCofy8xsNXJ6mjrWCtNVKnUistbSVSMiwAMqLIbpVGHGQ6eC9HeUueK3qV9lPCzkMgwaTlw7vYvSaM2r0VGcu2rMNmkVnykttV03hXET1pG4izikgYNA19qt6A2u4gMAkmXB4zPBTXrcfLppaKmzZP4DMFi8drAvV7VhhZbddLP3ERaIBj+HEoCL1pzio4E0zcBUryAocew9FoVGo/JYKrzj49PMVFirmRt5CLfouT/XoxcA8XHBXRauadI2nwafbyMATcCvoDNrTNbgOIzGA26CPwbmkXKUylh+ZCzvMEQny7LGYwcEVtg0cBcXckjIMo6X6h5uk6mfMjYArsCqXVYC9L+iPnyJs8G9yuYw/u7d0Mwmp82fH5HmUpN6ivqAg21BabLGuN/5AtwYG7fns8oNKPou3XTka/s5s5C3Qxdq8Mx1WKjb3jaPW3YajDJr0e4tpIAuFeiQXSIp+/9qwYebcfMmxA3I77As/B3ieRQ85mTo3jAlVRC77pPo89zYwz h4k/KxQg QQJ62q+w7QAyiLgYq+717oVEXll7tCQorTdseAs72I3elNyHCtJxxsGrGRItAlTjAiw5hdKdvfhNrVknJwGyGR3InLuBDJoSoyAxhKqPwASQQDxPTPZzXKDsSwnUJ6Aifftcta/03tipdhgxi4vuEASR4ChdgPm6vFPleRfYNhiZgHGtrOmHl/XoOF7FG7RuxV1C08BFEr4J1FRPATv2jRtTy7YQiKrG5Tw/mLL6xjz2nGkw1oikJbx9zHYfkz4asP/BH0+vTNbfBOFih4uHRvYQDzKnh0A2jENihACBG7CgBEYdmkKA8EzcGeJIa3T08znuGLwBTxvucobkf9BchLwl4ygDI3OX3dsn292fs2wom/uPpLlCEYRi0QHCJUkWK2jEpAz10+h04TdUcs9lUIFctS+F87bBYRQzRv3wBUVXLO+3WlJ454RMpGqxI0fjahJcnFNpSrH2pWsUnBFYCPsaV6E/U/xXoTl1jtpN+6DDjF6lfURpGgfeL07a0UPCCW3dw6iyptVkUowwCYbyLPfsOf6JIfgrdMkgy4GRtEeTeLLsbCa6huKKPE+iGM6qyYHPKQtHi+H7QJFA= 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 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 l= ocking > >> > > was always wrong for readers (for writers it probably does make = sense > >> > > to have some lock - although practically speaking nobody cares t= here > >> > > 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 fr= om > > __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.