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 88A30C27C4F for ; Fri, 14 Jun 2024 02:36:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EC2F96B00C1; Thu, 13 Jun 2024 22:34:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E728D6B00C4; Thu, 13 Jun 2024 22:34:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D13656B00C5; Thu, 13 Jun 2024 22:34:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B49B86B00C1 for ; Thu, 13 Jun 2024 22:34:29 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 6D2DF1603E5 for ; Fri, 14 Jun 2024 02:34:29 +0000 (UTC) X-FDA: 82227925458.06.EE9B85A Received: from mail-yb1-f173.google.com (mail-yb1-f173.google.com [209.85.219.173]) by imf08.hostedemail.com (Postfix) with ESMTP id 9F18D160006 for ; Fri, 14 Jun 2024 02:34:27 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=EnmH8Cgo; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.173 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718332465; a=rsa-sha256; cv=none; b=btHSkfodonl89sVYi40zjC6HsHEOD+PEUTo8iK2cYwrrvsKke+/z8eZTnrwqa9f2+HEfon yLMCftKAX5YglZuEu1GN2Y+xbiPatxhUuXDFlAYln4b2gaXbzhGiw89US0Qy6JbCH0NqLe 9PPnKPcEOqAUjqp2AkmvEuUTHN8XhQc= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=EnmH8Cgo; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.219.173 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=1718332465; 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=WGt7k6r3FpcN6H9iB54r2UkgCkDa1oqmP1AFpUTryM4=; b=pGWliUq/FRp1TcWGvjQGOKqhoKCMWEr2suwcrDNpRAyQph2K8lOy5uazOGhbM4+8/aYxDS vbSpafrILEWmLoNsMA/QaPoUpnQEmkp2w41eYXhOJN7vjppJsjrAh5pKEDF9D7CQhPAq0w GcWZHHZnxYFv0Y33PJjkVFOBPRUGytg= Received: by mail-yb1-f173.google.com with SMTP id 3f1490d57ef6-dfab5f7e749so1828543276.0 for ; Thu, 13 Jun 2024 19:34:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718332467; x=1718937267; 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=WGt7k6r3FpcN6H9iB54r2UkgCkDa1oqmP1AFpUTryM4=; b=EnmH8CgoSQEULqttTbmDdKijCdSIIGGsW4aO0s+6kdG1MUtXHkg98vnFurvnBNTE5R 9TCbi20jVFI19+QWZLIVNrWclNzCD9SEX9OmDDY3o+Q8glaILwxyK0efcv6TLDs9//yE QK53TPRDz77ErnoACUunycVPwJI8VLgB8BqMn9x5PHbvoB7qt/gMSk3fOWFrm/pVG/00 Z/1/iHPxDT/+TRfFjHab3Sj6a65Gcv0Mf6VNQEE9gMhLQqAEzXgWKSawGvsgxOUiCuzw CiEb3Bi77u/dCUcp9mwc0ZuyvDOJI38hVvSotxv+6hSx22x42wo5kZyN+q95Yt+N5PWr dFYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718332467; x=1718937267; 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=WGt7k6r3FpcN6H9iB54r2UkgCkDa1oqmP1AFpUTryM4=; b=f6bFjSsu4mH1cMX66hCoLUPrbkM/XD45QRoaJxMsRE7rfaO21XjhRPlnnpIlBGQxLc nxMzqVQzt8EmGp/4Jv/WgYN8o8Gzouobqrh4DZidfJu5v+ik36jrIalka01/1JAEeov3 5RcTXCCkNlYdBGdPR6l0gGu/TspqBtJrA/9vI65UIfaUBWwWzHdDuKfk8rLKYP0yd8/M Ov9fmdzTX79+c9Equ+aNkgBvrDeR7qUp/bLCZ6IF5hAoFkEsVLEwBjQOlx4srZjSNhsu KrnnwQN0zB0tbQc78o5PzqzE7VUBjoc25FXoJwMkc7Cd1jheRKhflLvxyg1JYQIDc/Sc Qm9Q== X-Forwarded-Encrypted: i=1; AJvYcCUnPJUZhMrmNSHbaNs5vgkSCK4rXN9sbthpCV6qOGdNjTtBee3dGqyfNyJ/z2EloXjI4+h9gB1rzLRQhUfbtRGqfnQ= X-Gm-Message-State: AOJu0YxcFdSK6oFb23NY6l7ZvqEK34JankeZU7EKGNDt7LdHN6inXqVZ K4/D3zfdG+aFaKmu43Qfc676ndHADdQxnwihJfEhhNvwCTdxo+okaFV8PNteehAKPFcD7yN1vmV 9hEZkPG5/ufw4Td//pSlQRrOX1+js4IT3dGwKCw== X-Google-Smtp-Source: AGHT+IF7Sz7NY6o3j4C4IPnMtJt7woKj9cH4kVPcujOZcn7xP0eRFnMx3IdWG7c4BrBfrjM411w0aCOam3Y1x/7rGQg= X-Received: by 2002:a25:acdc:0:b0:df4:45c1:f465 with SMTP id 3f1490d57ef6-dff154e5be2mr1023094276.62.1718332466522; Thu, 13 Jun 2024 19:34:26 -0700 (PDT) MIME-Version: 1.0 References: <20240613023044.45873-1-laoar.shao@gmail.com> <20240613023044.45873-6-laoar.shao@gmail.com> <20240613141435.fad09579c934dbb79a3086cc@linux-foundation.org> In-Reply-To: <20240613141435.fad09579c934dbb79a3086cc@linux-foundation.org> From: Yafang Shao Date: Fri, 14 Jun 2024 10:33:48 +0800 Message-ID: Subject: Re: [PATCH v2 05/10] mm/util: Fix possible race condition in kstrdup() To: Andrew Morton Cc: torvalds@linux-foundation.org, ebiederm@xmission.com, alexei.starovoitov@gmail.com, rostedt@goodmis.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, audit@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, bpf@vger.kernel.org, netdev@vger.kernel.org, dri-devel@lists.freedesktop.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 9F18D160006 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: snc7pqytxdzwx6zr9toqcgtik8skf8bz X-HE-Tag: 1718332467-910060 X-HE-Meta: U2FsdGVkX1/1EVE0ILnvhraYLgdU+b26BjQbTeI5k3p2CCzNDZGvuMbjERJG/qFlLO1cZg5NBxs3YkKGLGrfGDf7E8UVd8f5oINZzhipxzwtMEwkw64vAWxEUK2v7jnyu7dtpTDqd1TK0MejRw2f9o43vn0VAqSlZZrMPvwPCx+J7OH3yleVi+lt/suUVzB37NGV3UNwK/y7Li2QsZoK5qSuBJPAiWfiSqh911EdhS5mLSUue20G8QvTFmHL/7YtGjmZwGnGLYeCvYPTnJCRvIDhsonGTwTLLmNfgJ7B77tbymhBBomZ3KEksRIsSAncPVZbuSRI4Tp/g97SxlIIZsE0QGKnB6H5kNqqz0DUCu8FFWEYIENx6lOZkSB1oZCr21eDB0ZK8Z8AAset/BZ0l0bIRVnT35kWqrxud6QKnPzg/2CIdZJDEGGGh0t2QGZncPL5S2Y3hyuU175aIIGAQqrBtO13dNmjCLkUwFIuzS8ueJXshA3/IbmyreceUeeLogM6oQAV0TkhVorMf708Bj7uurTItrfvjSOxMZV6wHO/saQ418tEON06tMEYiDPI5LfFF5Jzmr4hRaz97UBDV4wJHbEsETaKOc7nFYblLZUNUEqVqJtpJLgs356+a4Xawx+1R1cNXYVLsDXMvXdHJnxszeUCpg3GEXk++UTfYtoYl04GlEaGXlA0ujiQ+H2sJu26EoxleAnmv96DWozRA6ACUAcIhV6uQWCtDMn8BZfpZKb750x0IEAhApeHQ/oWbHG8xhfGIcEwSQk5QwJ1cwOwjKgxlA2nXKnhNgMzcTWUCl8sThTQMNE0J49ICV6pYgA/GCLrVq5hf6Oo868L2G1PG5MgpROY2kbyvZpVaqswq+r6T31fMiWbz2ZDLDdDBN9cIXBAe5G36hLDWajxx283PlW+53fsKCW+Lr4oeXl0kAUMyndbzOgozVvC0DRzIclL9ivheHP9VMcR5RB RKhXgUcs b6VoSH9yzM3I71wblBKtNSDJ9+7ozuUFKsDSNJ/64bgB6vxPV9+uL6eLNaCxieOPwDqC6NASXxhQS7unNcqPwfG/wY0GxfTjpJFU5kBCl8QxZun2ka6nxzvUCc80fdH+InuHqjPSZYzcytsfpS4sq8M4Xt1roiAgDzSXaYFNjeaJL/BtMfGZHD6QBMSyJduvXqDsR+ijyxu4kQYaT86deiUijV/zgYMZRmQvZuodoYrTJ03/R1KB3MV/72FxpEqV4+4Xdd8lplztwfC6tuTYNJWTyUbKVBUSx5PqLFQMC0mACzgeuHD9GMNwT+WdWFnZFwsNR2DVY97w6iPQTJxmwsyzHzXOwa00ADEcZEvA7cTBeuTwajeiG33QfKSt3vSsrZYVpTEHhM0LZ2XaWaWCKqHb8bcMtpV5uQaXmNzjoux1W1D7ecdl50CopJk93YEbt4Grv0GNFe3oQ4iYXzU3IbSy/pRh2pvLXHhvuax5x5heIUfY= 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 Fri, Jun 14, 2024 at 5:14=E2=80=AFAM Andrew Morton wrote: > > On Thu, 13 Jun 2024 10:30:39 +0800 Yafang Shao wro= te: > > > In kstrdup(), it is critical to ensure that the dest string is always > > NUL-terminated. However, potential race condidtion can occur between a > > writer and a reader. > > > > Consider the following scenario involving task->comm: > > > > reader writer > > > > len =3D strlen(s) + 1; > > strlcpy(tsk->comm, buf, sizeof(tsk->comm))= ; > > memcpy(buf, s, len); > > > > In this case, there is a race condition between the reader and the > > writer. The reader calculate the length of the string `s` based on the > > old value of task->comm. However, during the memcpy(), the string `s` > > might be updated by the writer to a new value of task->comm. > > > > If the new task->comm is larger than the old one, the `buf` might not b= e > > NUL-terminated. This can lead to undefined behavior and potential > > security vulnerabilities. > > > > Let's fix it by explicitly adding a NUL-terminator. > > The concept sounds a little strange. If some code takes a copy of a > string while some other code is altering it, yes, the result will be a > mess. This is why get_task_comm() exists, and why it uses locking. > > I get that "your copy is a mess" is less serious than "your string > isn't null-terminated" but still. Whichever outcome we get, the > calling code is buggy and should be fixed. > > Are there any other problematic scenarios we're defending against here? > > > > > --- a/mm/util.c > > +++ b/mm/util.c > > @@ -60,8 +60,10 @@ char *kstrdup(const char *s, gfp_t gfp) > > > > len =3D strlen(s) + 1; > > buf =3D kmalloc_track_caller(len, gfp); > > - if (buf) > > + if (buf) { > > memcpy(buf, s, len); > > + buf[len - 1] =3D '\0'; > > + } > > return buf; > > } > > Now I'll start receiving patches to remove this again. Let's have a > code comment please. I will add a comment for it. > > And kstrdup() is now looking awfully similar to kstrndup(). Perhaps > there's a way to reduce duplication? Yes, I believe we can add a common helper for them : static char *__kstrndup(const char *s, size_t max, gfp_t gfp) --=20 Regards Yafang