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 74CFACF6495 for ; Sun, 29 Sep 2024 07:58:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8D7926B026D; Sun, 29 Sep 2024 03:58:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 839856B026E; Sun, 29 Sep 2024 03:58:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6B3146B028C; Sun, 29 Sep 2024 03:58:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 4A5A76B026D for ; Sun, 29 Sep 2024 03:58:34 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8A02512179C for ; Sun, 29 Sep 2024 07:58:33 +0000 (UTC) X-FDA: 82617023706.12.E56FEF9 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf06.hostedemail.com (Postfix) with ESMTP id E2332180004 for ; Sun, 29 Sep 2024 07:58:31 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="na/tpird"; spf=pass (imf06.hostedemail.com: domain of alx@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=alx@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1727596674; 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=nBLN6o6lCJRhJSQFvIn3BcrQdfxfwe8cw2FTKGHqHhw=; b=KvivIMrJyg3eK8JjrYZvZQBAariatBZERpBPIj3m8OIkFOIGZ3thkqjhRcFw/RbaEZ9JEK wrnAM2luHZOnWfyI/8ukruiqk+T7LqRdF2QzIGZHzxtzHjXENF8QHssCz6D/h2urM+fu0F 4wmb0EaeNwU3s+cePXWgaCRXYjuVwIg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="na/tpird"; spf=pass (imf06.hostedemail.com: domain of alx@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=alx@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727596674; a=rsa-sha256; cv=none; b=bP2xudtOqFBZ+m2cL9zBb7KmeWUg2/lmd8O66ts3cqcdyujqyVWwF2NDUqb1kdLhjDyW6/ cUdlAanBf+DSFTQiILU1AX5Fy3BdtL2vCIEbrqdRIdJ1oLOq5Dj/h5eB/utqfiZenQYEhr ZNT6iELUgT4u9RgeoMrTNcQHdaGX7Fk= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 9F698A40170; Sun, 29 Sep 2024 07:58:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DC4E7C4CEC5; Sun, 29 Sep 2024 07:58:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727596710; bh=3xHAO/1nMaKipfHebjoHo1sKkeBUhtCQGPDbex00eno=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=na/tpird1bl9uvUKD9qDM4ARwhFA8PdpOm4dQQ7LxTWB/c33ja9fLlbqblYaKveqp Dd1mL5wLQhBBHY5REMFl7wDVk2vg5in81Z/qrjqgroWrMr3iBR0QeIK9xo2g+fID1W fZItj8cABW/yqR2e/9FCQSn2af9tg6wehIBneqHIGm7ROS54HNnV/IcC74/wYenXJj tccFdIJcBXl3O9NWHn38NKksr88Yq2jp0vqBFnpjaGrdNMZbnTJOOK8iAiy+HshUZi 5744M39jfLtZQvSe3QV3UBHhc4ZP6RD/28eA0H8XD1lcVIHekhInDLtSvj5jVVewoi 1xkMVvPzd8Ktg== Date: Sun, 29 Sep 2024 09:58:24 +0200 From: Alejandro Colomar To: Kees Cook Cc: Yafang Shao , 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, Andy Shevchenko , "Gustavo A. R. Silva" 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> <202409281414.487BFDAB@keescook> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="sc2ag33lzzfktmx5" Content-Disposition: inline In-Reply-To: <202409281414.487BFDAB@keescook> X-Rspam-User: X-Stat-Signature: oy67zcda5gzyq1xqtu18uhfryy8sz7df X-Rspamd-Queue-Id: E2332180004 X-Rspamd-Server: rspam11 X-HE-Tag: 1727596711-295737 X-HE-Meta: U2FsdGVkX19qkUwgFclR2Cht4eNJoiUssJUAA43qXHY/uL25fepyPQ978uapNE1jx5mXAyrywYJYKIEjloYRQ99IqZQTVMzB5Y4LClwEpW7XAcXI+HERJ0XRs/wcnwxURmNQwb3Pkf/Q+D4C9X/a8Og73cDudUxljVw5DR3cAJ9sJLKZm9ztQTptxjUtcQDnS4OpxuXeSCFTMxkwtIhbulpmtBjuOkhHlYbjSy0G+yWVHhJ1FsDTqJ1UvHNcp8BDGC8fLcglFo3//lvMpvNjVvQMzbMNx2xbCjzRjfqkva51GkLVkS/MWqd2ITq8ycKIA1yessG+Imb+BWNqwCCzsUFrBzpGlUplkyKhnEEVfS6OCvPHYxQAzruED6OMcR6HB+K4O93wG9tZnojwajd+yeWmqe9467FqyadXoYFQKMCOBzgZsd64qGOsVfrtxEaBgMDH3aIUXNBVdeObeEDKrRrgpo9EmYZ2w/o1pTDaGJbToUG4mvTmRITnKEUpsgyRm2sQvohTCWPh4Agmib5Sv6j26hTfdh6J2FubkqcXjukgI+C1dTl9euQ0RUEcUqFbgz8GRSNW0bUPIvfiCbgM0/bFUgcsK8yJ03/i3GT5pNOuPVT/t0hU2nB47aHwXYBdR+vQm+JHZYjgeSpCec7peCIq8+gCAjrtVeAveUDiMdLCAaV5WO7ZwjNl8O3+HSazH1LmAsc5JN/wic3Mw47Nij/hlWS//6sBFUjtxQTlRKMl/nfJHAZnQf1Mrv5xogA2MBpRuDBwhI3IN8aEKTS8spaI8zQzy4H3/FOteDY01ZFKPsfRDPoqkwH9u/WWQQXvnF/3WGZ0ZyBVdadp2O/Iiu8dUtNWOAQRflk/zD83OObOYSsRibrSrv3EoWOZZqeTUZ5VU2Jv2OiHiTV4x/MtQTHh1AliuLU7tmW1uNeQKb0Y/3vti84bU3TspDoKTKGfFPizQYHJg3ROjfFyYo4 Uqw35VBR nQcvr30qrpWS5uM5eEVmnmap9b+Y0uIwug1y034BXuEeqYyEsj7NSgXE60hFOKw6uPOEUHyNWOSeS6BGGss2Hqo13UDb+P6Fst/LfCTsf3DHC4gum6g2dtm1WW8ValsnXwQlQkjw/xrYI7S65HxOOD5rE0nvr3WOYoU7ID6JPsFRYPW6ZaUuWscQVUbneOo18m00fWnNtYzIzh7YJ5ZEiWgpYFNPJUM8nfh+IeZuY4+9ov1rjsxEFcpACEb0wgR6XKP6pQ1ckgox9B9ZhaQDIvUjRIQMya+tjti547QvMq8d75mPEDDXM9zw33OwBGdilCP8CTNdyxNglzAGbbCObmupi8QqTVHqJe+Zc0zC7pRJXqWDK2jfh9eGVKI3AiL/OVPWKJ0V9nqeHkZmGRig13bnb6fNTimZQksdPD+Fk/mrfBonogWjZP+UL3oaOWRVgyKPFjX6TsiLf2E8Y4aOO4sePGm+AADGvFRTz1Vl/uvvRRMe75CwEH5V0O+zBBPEXK967AbnFFcn3Bv9CA5P322Dx9PIRpPSHE4ptojHLFAzS02tCa71ErhMFkyRCIbbDuqsyB1ErObYecY5Ys7KDjadJSQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000096, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: --sc2ag33lzzfktmx5 Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable From: Alejandro Colomar To: Kees Cook Cc: Yafang Shao , 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, Andy Shevchenko , "Gustavo A. R. Silva" 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> <202409281414.487BFDAB@keescook> MIME-Version: 1.0 In-Reply-To: <202409281414.487BFDAB@keescook> [CC +=3D Andy, Gustavo] On Sat, Sep 28, 2024 at 02:17:30PM GMT, Kees Cook wrote: > > > 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'; > > > + } > >=20 > > I would compact the above to: > >=20 > > len =3D strlen(s); > > buf =3D kmalloc_track_caller(len + 1, gfp); > > if (buf) > > strcpy(mempcpy(buf, s, len), ""); > >=20 > > 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:) > >=20 > > #define mempcpy(d, s, n) (memcpy(d, s, n) + n) > >=20 > > 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). >=20 > Just to repeat what's already been said: no, please, don't complicate > this with yet more wrappers. And I really don't want to add more str/mem > variants -- we're working really hard to _remove_ them. :P Hi Kees, I assume by "[no] more str/mem variants" you're referring to mempcpy(3). mempcpy(3) is a libc function available in several systems (at least glibc, musl, FreeBSD, and NetBSD). It's not in POSIX nor in OpenBSD, but it's relatively widely available. Availability is probably pointless to the kernel, but I mention it because it's not something random I came up with, but rather something that several projects have found useful. I find it quite useful to copy the non-zero part of a string. See string_copying(7). Regarding "we're working really hard to remove them [mem/str wrappers]", I think it's more like removing those that are prone to misuse, not just blinly reducing the amount of wrappers. Some of them are really useful. I've done a randomized search of kernel code, and found several places where mempcpy(3) would be useful for simplifying code: =2E/drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(pwps_ie, pwps_ie_src, = wps_ielen + 2); =2E/drivers/staging/rtl8723bs/core/rtw_ap.c- pwps_ie +=3D (wps_ielen+2); equivalent to: pwps_ie =3D mempcpy(pwps_ie, pwps_ie_src, wps_ielen + 2); =2E/drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(supportRate + supportR= ateNum, p + 2, ie_len); =2E/drivers/staging/rtl8723bs/core/rtw_ap.c- supportRateNum +=3D ie_len; equivalent to: supportRateNum =3D mempcpy(supportRate + supportRateNum, p + 2, ie_len); =2E/drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(dst_ie, &tim_bitmap_le= , 2); =2E/drivers/staging/rtl8723bs/core/rtw_ap.c- dst_ie +=3D 2; equivalent to: dst_ie =3D mempcpy(dst_ie, &tim_bitmap_le, 2); And there are many cases like this. Using mempcpy(3) would make this pattern less repetitive. Have a lovely day! Alex --=20 --sc2ag33lzzfktmx5 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmb5CJoACgkQnowa+77/ 2zLfmw/9FsLN/M7ZkpM7L+8hpOThHHZDCRD40jrK8GQ9Ao1lmIKMASXMncuGw7Qn BoGLg+9glMCF47rsNtrqU5iSAoXuXOhIbi6iJUxF6WbK/Y0h4un0vjBoBZuoINnP fnGIPzVp2pNx70EaOw1Af3zUpXpbdzJYFpI++i7OJ4dnH8uQ5sbZMs2HioBKiWRT arfK0OD+HhJHE7GRtZMMCPeq1JvaELpBPp2exwe6j29Js6cD0EX4T9cLf7zTzU2n 4enMj6OY04Hq78bmLv2Ej13DHYSrQCQdcbYu5auFN/dF3oq5AuB8XAk6L/gnuXdH bxIsS3yRvZL3JRYRU/n9RJzzAlrUX7wFo1/EVQFQvw/tbhmizOL3UM4IW8AXTxX+ b4UuHBu+U3bGx1xCREqqWdq0Kl7CaGR2y8HipW5BXRa+58CaqZd3KPiyCvxsFxkN mMHgXRagtAo/RAjPapyBy++yBNFAy1QiXs4C+WOyONP3x+AA0e+tZiNvZOgFSrHC 82An0a78d7f+1EXhpuE8X+LpVqwR7EFQVnPG1ox9B3B4380hphULOEa9HqstIvvt WGCtOL6T/Jb7SVoYesDuu26eQFoiK4JmDJht/K/z7NEhJsop3qzKnEnKl81GeX4K sNCQWo2X1SFnUZcxvQKNzGVMsOlNdjjTO4pOoSEFgurln1kI+BY= =4tYC -----END PGP SIGNATURE----- --sc2ag33lzzfktmx5--