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 DB889CF6495 for ; Sun, 29 Sep 2024 09:48:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 156C08D000A; Sun, 29 Sep 2024 05:48:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 107338D0002; Sun, 29 Sep 2024 05:48:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EE9FB8D000A; Sun, 29 Sep 2024 05:48:14 -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 CCCEA8D0002 for ; Sun, 29 Sep 2024 05:48:14 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0EFC1C1964 for ; Sun, 29 Sep 2024 09:48:14 +0000 (UTC) X-FDA: 82617300108.15.F2F36EC Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf21.hostedemail.com (Postfix) with ESMTP id 644261C0009 for ; Sun, 29 Sep 2024 09:48:12 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ZZYqjUkd; spf=pass (imf21.hostedemail.com: domain of alx@kernel.org designates 139.178.84.217 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=1727603167; 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=Y3KP0/ZzPGl7S6XQtQCMwSFvMXxconK+yMLx/S+3P3o=; b=duIg6CtbStofRaH/clyDT29s7EmCW3UUrk4Hj61G9x5ScXlOB+tJRvr+9BpT6dq1mpnOgT 0JzY6LQW4t3FWkgDeu97+F44HrszFmsaRV76bT9GZy79aOKC0UmpqShjTJje4VYvWbN8qx w7JRLFxxKpzzLWs9L1is6I3OpONlyM0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1727603167; a=rsa-sha256; cv=none; b=58LKwqJOujIzzFQzxREtFF34ymo0AHbGX0jsLwwXNdNslEpxmTsRCrLKNoTy+935IMm59s oajHSe1CkDaY+HIZTpN84Ap3Neszfzid0dmatRYooSsZVqyQD84xCMxAlQysoxhMDU0AxR 8ce+DSVWDW9is9N9t4VVLYCU11HjGYQ= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=ZZYqjUkd; spf=pass (imf21.hostedemail.com: domain of alx@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=alx@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 524355C370B; Sun, 29 Sep 2024 09:48:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5AAA7C4CEC5; Sun, 29 Sep 2024 09:48:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1727603291; bh=Q85EQZ1C0LWdCS5RaobjsfqzLm5bDoBuzWbpeweMj9I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZZYqjUkdiv7S8FgTbaYbV0XUknhL1P1VVmTo9P1W2GctlLPXcJ8oJQHBKlPZgjXo2 Es2xS3ESgdg86l3wNftoKH9yH1/mTm6moYrm8dO7zU9boWmnMHAw8RgfZX0mjjOtZF 3Topn8mc6AxsEl9nm1AzniZzWYp8ZvUI1lQvJdgHq4cipjPYGwJMkUqiJAjl1fNVVH 4V2xLeDXnFXCRYKaPkvjwJu2MSGKPGUGkuP+NA3eC/XdQPRwLsGdFWj7FNnY4DOSXy CKQblIkIkEUIRFJqSL4eUbxg5R1gDF2r/Fs5YYJQanEMuWLUSlYPW9HSAKjbigtLm5 iX9v7pf55JwHQ== Date: Sun, 29 Sep 2024 11:48:05 +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="vmgihngka57rzukx" Content-Disposition: inline In-Reply-To: X-Stat-Signature: zmkttda5nuy4enbe6c51ochpa7196rhu X-Rspamd-Queue-Id: 644261C0009 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1727603292-326817 X-HE-Meta: U2FsdGVkX1+HRQ1F721p609CuOZ6w4t71mgrertmx2nqVLheCJ5UPL2gKvGHNzMjHoUPsYK3ta2v/0XRzVqGjnZE8oGWLEuJxU7qB1PMoAZYyyNntpnqyiVpB7IQB8AO/A8bnr8t7kX+isULiwE7Y6SnZTHkBTzvDYfej0UeUs6FjgrkMYtgYGk6xDN+wAP/10keIEmbNn6Zl0PDParC+bCXTehYSD1g1k7czcrCDqIebmi0xAMkx9ADr6GCfDGPNhTyLB6U56hAdM8BUjTlIku4yW2M7aU5wlWoxxZ1g6P5I4y06MprtCerdqpZZDqqix8ZUhe3SUMGtYD+V7qcoBL7/qa7ewIl5rvU3GSnQxZTKd/eI7gaHuDYaXPJTa1jQyiCqsvhSzKlmOjM1nZIki4z1C+qs1/oHq4Aciit/itUO+ThQoSLLm4JYnVYIl/cms+h6IsWwzVNjgDOzA+RCies6n98CFp4edtDeG4mNf/qaCw0KIWpzBm8z+3ShAL0jfkEebeaz4pLXSd6ARZrcZa/TAWTLubar43a5QTNd0j3PX9ZrsAEkBZDaOF6MsccNS1tEPnm8f49asfjnMLYgh1b8ZGwEZGnRJ6xaERcT7H/6dSGu64iHZ5eapxJVZBN6F1tfrb3/U2amxyVjNfB16Zl4knuxMknMERI6fRIWt9NKARYCEM9+q3cpRLLJW5HRmnhwYwCBD9uaOfvbNDOpbGRSrfbqy/MCkJtVHFDkttNFW1wsv6jNf8GO5gQBwvm1jiFkBG6/SVN3Vk4XM0HiCb3tCjFgYvXQDvcb/bVPlCoNx5G3ahHeUYmOYxKgO6aTV9GIN760DAUfdpjA3z5rJEMRjPKD6oEkWqAnf4R53abeUtIHY60o2CkDbZgApkFQoUsPYtGEsHEyANCy0rTDC39dyWxTsJTvjqYzgHe02iWdN0LYROxCyIzQQkCJDntWFc9kdei5+vBtIDYCX2 cqngAsoC aVmfj6u7OeQJC+u0XbdQvBhgPGtUW0Z8RCsrCVUvrxOPItCFTSGb88jYY4BTKv213j8JD2+GA2Ofi081JUGmPaQSzf/cCCQUoyo3mUYel609Og7XQe4mTrZySkYpl6qfpMW7L3K/25s+Y3V0SZ8XIaGT3OFCDvYdBApx66CGaIVW/eloUf5nGeMGDwwLPyLwV+QTF0u7pIBV6iqgvBWr3COezakBLjdDWPGSk9wkS/hwc3bC+vxuAAI+Of+AVKU9mIE8cWivq4Mjz9vZ3RaJPAgTbT8uGqJV2BLby/9DUOLxVsq91/a560tM+HUA7QUSfDP+i0p8gHhcIKbZ2wZ0veIl8kODz6cZ4oMwCjTjieGxPSa/HxrvFRMnoOnCmjosUcm8ERXJmS85l9XO6zOjNcMTS5y71irYOtzVwzyhzTpKJNr7XAb23z271IrRXtjiAdDf1Se5UEH+lzZcklKTL7FvQY+hg7ln1dBpmZA0Ie1tCY4kQJN/FZbLrXBAHV9DCipLPROMZLHGfwZqk4SOmzdmzXuCvdwRS2YZueFrrT10xl1PUSk2H5STNwB/LHkAUJGxvTwOa+FHXF+r4iN4sU34EZg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000065, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: --vmgihngka57rzukx 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: On Sun, Sep 29, 2024 at 09:58:30AM GMT, Alejandro Colomar wrote: > [CC +=3D Andy, Gustavo] >=20 > 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 >=20 > Hi Kees, >=20 > I assume by "[no] more str/mem variants" you're referring to mempcpy(3). >=20 > 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). > >=20 > 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. >=20 > I've done a randomized search of kernel code, and found several places > where mempcpy(3) would be useful for simplifying code: >=20 > ./drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(pwps_ie, pwps_ie_src, = wps_ielen + 2); > ./drivers/staging/rtl8723bs/core/rtw_ap.c- pwps_ie +=3D (wps_ielen+2); >=20 > equivalent to: >=20 > pwps_ie =3D mempcpy(pwps_ie, pwps_ie_src, wps_ielen + 2); >=20 > ./drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(supportRate + supportR= ateNum, p + 2, ie_len); > ./drivers/staging/rtl8723bs/core/rtw_ap.c- supportRateNum +=3D ie_len; >=20 > equivalent to: >=20 > supportRateNum =3D mempcpy(supportRate + supportRateNum, p + 2, ie_len); Oops, I misread the original in the above. I didn't notice that the +=3D is being done on the count, not the pointer. The other equivalences are good, though. >=20 > ./drivers/staging/rtl8723bs/core/rtw_ap.c: memcpy(dst_ie, &tim_bitmap_le= , 2); > ./drivers/staging/rtl8723bs/core/rtw_ap.c- dst_ie +=3D 2; >=20 > equivalent to: >=20 > dst_ie =3D mempcpy(dst_ie, &tim_bitmap_le, 2); >=20 >=20 > And there are many cases like this. Using mempcpy(3) would make this > pattern less repetitive. --=20 --vmgihngka57rzukx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEE6jqH8KTroDDkXfJAnowa+77/2zIFAmb5Ik4ACgkQnowa+77/ 2zKNqBAAqNB/fweA49pRRWO/XgFk0EvMiezUqWV2wXsV2l/VUrjeGx/Tecpvhh5b cze6CNkVl/fKu2M8a5/ysj/r99gtHpNA85bmby3/b4BFwXNfPd7V+Qz1hUI7iXom 3hnuyFNPMH7iIyUxvwwHM3k3qSh1G/rxcVJm2NkRDC0WJo7dEIHl8UxaaXTEcqTO bbb+umzXIosV+Fhsk1tjwgORKj5GaOVMbbF+hqFeuLt7z1eFWb3MYQAavRKN7SNm QVgnMlJZir6XIzQv8HaAQvulm4x+N9xvMSlQ9ihADbMHc0YZPxp/ZZqYmnvyopWn 5FlXrWZhfi+tbHSciR4f/n8OVg6go0483iVrLZNs8+rne7Et8iExHCqHlqAIlA9S d1hJfdjZGGdgO5DbA6Y3H0hf/XS9e/IZ+a+T0MEdvjDv7pv+zhYSuapXnbJ+xbDB XmpEadusEMQGWkf+8GEeXiI8IujrMCAlncDoisghL3BF/ALac9zkWnFQjjW5l+xJ op+juVgs6urGNtdRzW8Itu+AlkSJyuYb0rbRfoE6NN+yRG04Yxf9rMC98bI3q8zl ZZr85yLJayTBLOD+QQVsbeUQJFs4sJan+Bj8EN42+BCZ+Se1XCxSeh2nDSXlpAAX wv+032Rpbz/aJVCJLO2r02aT/nYCQBDo/gNYqy/cMqdm8R35R80= =kmHt -----END PGP SIGNATURE----- --vmgihngka57rzukx--