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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 386E2C636CC for ; Wed, 8 Feb 2023 05:48:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230358AbjBHFso (ORCPT ); Wed, 8 Feb 2023 00:48:44 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39838 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230347AbjBHFsk (ORCPT ); Wed, 8 Feb 2023 00:48:40 -0500 Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org [IPv6:2001:67c:2050:0:465::101]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B251911EAF; Tue, 7 Feb 2023 21:48:39 -0800 (PST) Received: from smtp1.mailbox.org (smtp1.mailbox.org [10.196.197.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4PBTZB5KfVz9sNW; Wed, 8 Feb 2023 06:48:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cyphar.com; s=MBO0001; t=1675835314; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ZylWv+h3Xu218ZbfmC9cFiHzhi6vfLtl/EmguZL1KJs=; b=UnzHsKqUwvCYeRVg5A6EmHxZ8pout8XsTe8f/O2sayi18pZVmHHbqGy3IpvxaabJuYFAE2 Ypflp+2CG9ATBGTRU32rdh9OB6ATHhzFr8+S2e7w7JZtV1GCq0sU5h1THC1VV2ZksOr8r3 cbk1BcF0C/pnLjEqd52y8EQH0pKsvbCw9D2gWTNL6OVjNcfHY0A/PuJA379BonOWbEuiRC qd+6NeKmthFLtkJPt0BECDW+neq+NsEudIjBruTIckjY2q+p2iXg0fyh+Rp0osai5qQDcY fVwfDMB/rDDPVg6+zBrZchjGchnF8osOapA7gLRTo6xyX4t8MM0YsWbWZAa1sQ== Date: Wed, 8 Feb 2023 16:48:18 +1100 From: Aleksa Sarai To: Kees Cook Cc: Arnd Bergmann , Christian Brauner , Rasmus Villemoes , Dinh Nguyen , Catalin Marinas , Andrew Morton , Geert Uytterhoeven , Alexander Potapenko , Christian Brauner , Stafford Horne , Al Viro , Christophe Leroy , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size Message-ID: <20230208054818.geaw2toki4mo32uc@senku> References: <20230203193523.never.667-kees@kernel.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="vp6wzq7tucbqsbdg" Content-Disposition: inline In-Reply-To: <20230203193523.never.667-kees@kernel.org> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --vp6wzq7tucbqsbdg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 2023-02-03, Kees Cook wrote: > While there is logic about the difference between ksize and usize, > copy_struct_from_user() didn't check the size of the destination buffer > (when it was known) against ksize. Add this check so there is an upper > bounds check on the possible memset() call, otherwise lower bounds > checks made by callers will trigger bounds warnings under -Warray-bounds. > Seen under GCC 13: >=20 > In function 'copy_struct_from_user', > inlined from 'iommufd_fops_ioctl' at > ../drivers/iommu/iommufd/main.c:333:8: > ../include/linux/fortify-string.h:59:33: warning: '__builtin_memset' offs= et [57, 4294967294] is out of the bounds [0, 56] of object 'buf' with type = 'union ucmd_buffer' [-Warray-bounds=3D] > 59 | #define __underlying_memset __builtin_memset > | ^ > ../include/linux/fortify-string.h:453:9: note: in expansion of macro '__u= nderlying_memset' > 453 | __underlying_memset(p, c, __fortify_size); \ > | ^~~~~~~~~~~~~~~~~~~ > ../include/linux/fortify-string.h:461:25: note: in expansion of macro '__= fortify_memset_chk' > 461 | #define memset(p, c, s) __fortify_memset_chk(p, c, s, \ > | ^~~~~~~~~~~~~~~~~~~~ > ../include/linux/uaccess.h:334:17: note: in expansion of macro 'memset' > 334 | memset(dst + size, 0, rest); > | ^~~~~~ > ../drivers/iommu/iommufd/main.c: In function 'iommufd_fops_ioctl': > ../drivers/iommu/iommufd/main.c:311:27: note: 'buf' declared here > 311 | union ucmd_buffer buf; > | ^~~ Makes sense. Acked-by: Aleksa Sarai >=20 > Cc: Aleksa Sarai > Cc: Christian Brauner > Cc: Rasmus Villemoes > Cc: Arnd Bergmann > Cc: Dinh Nguyen > Cc: Catalin Marinas > Cc: Andrew Morton > Cc: Geert Uytterhoeven > Cc: Alexander Potapenko > Signed-off-by: Kees Cook > --- > include/linux/uaccess.h | 4 ++++ > 1 file changed, 4 insertions(+) >=20 > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index afb18f198843..ab9728138ad6 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -329,6 +329,10 @@ copy_struct_from_user(void *dst, size_t ksize, const= void __user *src, > size_t size =3D min(ksize, usize); > size_t rest =3D max(ksize, usize) - size; > =20 > + /* Double check if ksize is larger than a known object size. */ > + if (WARN_ON_ONCE(ksize > __builtin_object_size(dst, 1))) > + return -E2BIG; > + > /* Deal with trailing bytes. */ > if (usize < ksize) { > memset(dst + size, 0, rest); > --=20 > 2.34.1 >=20 --=20 Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH --vp6wzq7tucbqsbdg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYKAB0WIQS2TklVsp+j1GPyqQYol/rSt+lEbwUCY+M3ngAKCRAol/rSt+lE b0q+AP9OwsMxuge5vMYJo2+9fH5bmYNcLWHfBb+uD+aeLntahgEA4v8J1lQpM7Ki Qo4oyu4BaUei9uD+gINFKY2t7rX1SgI= =xVlD -----END PGP SIGNATURE----- --vp6wzq7tucbqsbdg--