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 66A05C52D7F for ; Sat, 17 Aug 2024 08:48:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E66E48D00CB; Sat, 17 Aug 2024 04:48:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E15ED8D00B8; Sat, 17 Aug 2024 04:48:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D04A38D00CB; Sat, 17 Aug 2024 04:48:22 -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 B31D68D00B8 for ; Sat, 17 Aug 2024 04:48:22 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0793240528 for ; Sat, 17 Aug 2024 08:48:22 +0000 (UTC) X-FDA: 82461110844.02.57F6FA0 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf04.hostedemail.com (Postfix) with ESMTP id B20FD40003 for ; Sat, 17 Aug 2024 08:48:19 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Uwag4ecI; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of alx@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=alx@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723884486; a=rsa-sha256; cv=none; b=7i3scQoC3V1r+dqq9VeCnY0sb7pnc8paIbVjxLfC9sPkVZfPPn+fYkVwjLJmeVW5aMErNo vfKHZhyAgE4oCU1EXGUVJtAk0HY7FHtHEw5J1St4gQlUEoGdTZDste6IzXDL4EtL88OkXO Ra5OcsKNnf8EONNbKdgNVgbmvC5Pg6g= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=Uwag4ecI; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf04.hostedemail.com: domain of alx@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=alx@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723884486; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=qiqXIjLVQE82iD83lBZv6AXThCZnYYlg1E+qOZnqiac=; b=0CdjEaHw5QfxPlNyFdX49QEq1mCXhdGzNpRtEQh1vrtlw01YT4lePTUdhNndEnSGfw6L84 dXKrsAVHOTgHIiX75zFX3en4w4S3ocgBmU390yBJrMWIGm2wmXC/Uc8goHIvuyNcOeaQ5m 7sjj2rNW/t1lzZ8bs6eX/N57AQsZ8eM= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 652D5CE0FFC; Sat, 17 Aug 2024 08:48:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4E68BC116B1; Sat, 17 Aug 2024 08:48:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723884495; bh=y+GQ801+56Qy38J6PlOE3jEeIZQ6gl38pFHqBSE07mM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Uwag4ecIv7SyLksRVrw758JAs/w9VL4kFVVtuawF0QJONmUWJh+UNbMist7MYW4h9 empQz2XHkr7lF8FAWA+gO+Jnp7942pie99CT7dmHDNGlegl0wjd7VhYhmAtlSURaqn HThZyMGPUVcvwKym2NQt8dviuYWyoGr96M8TRlRMph7s0c3KGiemmmVyR9zmax6vWE xoXf3nJ1dRAHRd3ndaOpOmiPJqOJJfxRkfmFkVqxCaaGxaqm/yDEwIilUyADUWcoyu uR7B8fBCl1v6MghLBxULfUB1erKub/VJ3504HgOlXkQV5Fz8z6T/y8S0nRrIjOq7Lh eZ/ZVgdVBJp8Q== Date: Sat, 17 Aug 2024 10:48:10 +0200 From: Alejandro Colomar To: Yafang Shao Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org, justinstitt@google.com, ebiederm@xmission.com, alexei.starovoitov@gmail.com, rostedt@goodmis.org, catalin.marinas@arm.com, penguin-kernel@i-love.sakura.ne.jp, 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 Subject: Re: [PATCH v7 5/8] mm/util: Fix possible race condition in kstrdup() Message-ID: References: <20240817025624.13157-1-laoar.shao@gmail.com> <20240817025624.13157-6-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="fmddhafpzo3fg7cn" Content-Disposition: inline In-Reply-To: <20240817025624.13157-6-laoar.shao@gmail.com> X-Rspam-User: X-Rspamd-Queue-Id: B20FD40003 X-Rspamd-Server: rspam01 X-Stat-Signature: p8dymsmsg7pbziqhuyczebhg9nioszrz X-HE-Tag: 1723884499-459904 X-HE-Meta: U2FsdGVkX19lVOtJMn0j2bPIc3qig5DuM4C67vKXnMG3LtLwNsRxAcaT4QHme1bptX0ZMDf7Q1nIhf+5xUKw2pUOTxCzuqxEg8cC82BsO9+pHoLyV3RQ5CwCzL5Rm86Jnqdq7K1uCcYA9/Yf5QS8olz0cb2L23J0zsnAvsQ6r5hOgGNn7lI5GcSuszjV6ui3tvwicVxy2NDDdQg1y+BtmyMjW0f/8atAyJdM04WA3KYrQMIIyWeX5Wt71WsAKrYRozGNelGUk97CyYs+C+G6xk8wzuhZ4S0HhWuK2hKL3McxcdKK8coQvvesdNNeA735JvZbU6jjZZH1X+OcBGUUhW6Z+jloJspnLPB6bggiCZnWLIJynyEcGFWRJY4hM1DXDawb6SxzlMtNm3SLyUOQdDbuhcX8+tMtAhwRoJs/T/AKgQHju5RgoazocWPObyR0y1mPOAkq3fkZAUF3o5l68PfXtkEF2TObvYmg/bO/KfWZQbpwhM3ml87ROJdwAr+aERW2d9mL10lgYCfmJ0GSOkoyIT0qn9lURy9XhQwmu0HAocAe51a4Yv56LgDPS6dacj1nyCMI9lMHWSikom0waYE7PAswPVhnkfMVHsh3hR1DRwASBNrtElO5gyJAXH0BrE9N6gaWJZU46fn7nrlEn24Tw24MUiEm8WbXD2E/6fKgGLZIl/Qv9ZkoDFN2/tX+Z2BCwOcjIa2uwBba0vc9h1m++RFECaBjyRsCvFb9k2nWnirtSbjPMMHFSYOgzzRNAJW1/aQs1jz4n0jMoneZluyOCKy0kW4/0IbDn6ZxMvaDAXxgPyUDZV/RrIUaC9hr6ywC7dAAEClSoSedNe10VyVhiyzfTxPD0Fo4JoyjeuM3bP+xIWQcAH2jHctyNlg7MSEp471IlnyC4Yei6vXVtPkTVi775F8Al+oVqVfKX0CePnj7ZPhZB81CoU/pRm03TeAKKN6pa0SrlQR2Jd6 6FxtivbY N95rGEREzQS+XbiRegYs+QtlEZNbCxyw8MCSgY3Dsd1B382bfTVN277kchm9Sdq/glFJgbW8hRT1wNcipn/tmKX8PvzxwXZ0ZnFs0yAh8KuPFNeQsHikix4zQVECYbN5Jn4Dcfn+wQc1IZGMsv3p/1CzfEVxC4Nj68kzzrAJZmcznB61dE7BUfQYSzb6d9w5Lj7VzLKZzs4VvoQITRgoLWxtxMM9sj5im8RR1NQgpCcfXhQnquH2bdZB/IJSgwDP79oKKAM2ufGH3SHpbJw2ImUVk47lspQMLKbhq/UNVqRT5+/LJMtopVDAQNaFIv7ZXer4AV08Y5bSNg8/VBtDIG2wgYPksCZlT0n/CtXwEqomdKACWvqsfJjglnQGy4+l0K+AWlDtXM7hDUqzBAgyWaFoz5+Hn++Um0olHqVfeWonQ364uUmgskGxXF29sOOj8Ge5PgY20mk2evyHHDoZVQHGg38B6Zfg1a4o3WXgPR7RYy+qXdY7Xtej2VeeZIRJmkdK6DxwYHx/Gv22trVR5O4gOGQ== 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: --fmddhafpzo3fg7cn Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable From: Alejandro Colomar To: Yafang Shao Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org, justinstitt@google.com, ebiederm@xmission.com, alexei.starovoitov@gmail.com, rostedt@goodmis.org, catalin.marinas@arm.com, penguin-kernel@i-love.sakura.ne.jp, 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 Subject: Re: [PATCH v7 5/8] mm/util: Fix possible race condition in kstrdup() References: <20240817025624.13157-1-laoar.shao@gmail.com> <20240817025624.13157-6-laoar.shao@gmail.com> MIME-Version: 1.0 In-Reply-To: <20240817025624.13157-6-laoar.shao@gmail.com> Hi Yafang, On Sat, Aug 17, 2024 at 10:56:21AM GMT, Yafang Shao wrote: > 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. >=20 > Consider the following scenario involving task->comm: >=20 > reader writer >=20 > len =3D strlen(s) + 1; > strlcpy(tsk->comm, buf, sizeof(tsk->comm)); > memcpy(buf, s, len); >=20 > 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. >=20 > If the new task->comm is larger than the old one, the `buf` might not be > NUL-terminated. This can lead to undefined behavior and potential > security vulnerabilities. >=20 > Let's fix it by explicitly adding a NUL-terminator. >=20 > Signed-off-by: Yafang Shao > Cc: Andrew Morton > --- > mm/util.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) >=20 > diff --git a/mm/util.c b/mm/util.c > index 983baf2bd675..4542d8a800d9 100644 > --- a/mm/util.c > +++ b/mm/util.c > @@ -62,8 +62,14 @@ char *kstrdup(const char *s, gfp_t gfp) > =20 > len =3D strlen(s) + 1; > buf =3D kmalloc_track_caller(len, gfp); > - if (buf) > + if (buf) { > memcpy(buf, s, len); > + /* During memcpy(), the string might be updated to a new value, > + * which could be longer than the string when strlen() is > + * called. Therefore, we need to add a null termimator. > + */ > + buf[len - 1] =3D '\0'; > + } I would compact the above to: len =3D strlen(s); buf =3D kmalloc_track_caller(len + 1, gfp); if (buf) strcpy(mempcpy(buf, s, len), ""); It allows _FORTIFY_SOURCE to track the copy of the NUL, and also uses less screen. It also has less moving parts. (You'd need to write a mempcpy() for the kernel, but that's as easy as the following:) #define mempcpy(d, s, n) (memcpy(d, s, n) + n) In shadow utils, I did a global replacement of all buf[...] =3D '\0'; by strcpy(..., "");. It ends up being optimized by the compiler to the same code (at least in the experiments I did). Have a lovely day! Alex > return buf; > } > EXPORT_SYMBOL(kstrdup); > --=20 > 2.43.5 >=20 --=20 --fmddhafpzo3fg7cn Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmbAY8kACgkQnowa+77/ 2zJy9w//VpNvNlz0qkO1e7GsXV2Oay7F3Mv07UYbMUQROCVAnkJ5089Gt5oQTGqt KofcZ8qVFOpVffW33J9WD2NmbWwDHudXlWSpsC98H4YlmmbpwZQOqTnA7OoZjn5V L8N5qXReoJTBCJ/nFO8FIu6FWYiZzb0yiIvDBdJ9wx3NvOVXaN9Id+YOU4OgzDBc HshTJLYpyK3iVgI2PEVf9nHnsgqqJaBLNCBQwqGkjO3eJJZBrpq+YcwB7JEbSzz1 q/ETj+JDUKBTCSQKKV1vGnrfe6/lQWfLA6fc9XHzIYlrwLeJsfXqnGyqSxSLRm45 HzYZbCiHqqIrIeK7EP6F3VrRvOFhT7pWKnopvysQ9LajuM8zWzHXOQuBXr1+M46O Uz5u/J9ZX9+ZzEmtgtSO9rIVGE4LfKd+ngF/gZCCYMobkUPet+e99uOhtIelLzD8 VviJU+xlRcLFPXMod7N/dfK6kfHxZ87K7KCetn68h2T5XDntslmoXXqed+W2Xi8p d6woLOoIM/AeQdMlzirWDhr/fXOG+X2+7ojkEpIK0fkv2bTWs5H0iu/qcoespWtJ ifPmabJGLvIN/8K/dDZKYNKKssIbS2ePuMpzghMbknKx3fQiB14LWaGUZ0fTzb8W YT4Ic9hLiqgcPLVyVEjw+qa28m+qnJ4nwMXDFbu08oqn/VVa9vE= =8JvT -----END PGP SIGNATURE----- --fmddhafpzo3fg7cn--