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 CAFF3C47258 for ; Wed, 31 Jan 2024 09:45:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4CAC86B0080; Wed, 31 Jan 2024 04:45:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 47BE86B0081; Wed, 31 Jan 2024 04:45:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 343926B0082; Wed, 31 Jan 2024 04:45:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 2125C6B0080 for ; Wed, 31 Jan 2024 04:45:00 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E038480C48 for ; Wed, 31 Jan 2024 09:44:59 +0000 (UTC) X-FDA: 81739122318.01.549535B Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by imf22.hostedemail.com (Postfix) with ESMTP id 23A5EC0017 for ; Wed, 31 Jan 2024 09:44:57 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=imBBIY9y; spf=pass (imf22.hostedemail.com: domain of urezki@gmail.com designates 209.85.208.175 as permitted sender) smtp.mailfrom=urezki@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706694298; 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=xqfllA/2YuYyJzutnMb4Kn7O4ZljTY6NQ9hi49SVfuU=; b=DiwUzG2hGzAq+TAuP2WheBczm0o6AleKE3LKN8mo4oEClXDX429jThoohjLsXu0Kpo7u4e Ngzvnk51McfUZUIZBazZHZhw7P0+n+fFjeHUht1PXEJeJrQa02LyiKTN+WGoYJGRushfbc JFT9EWkbSOX5c4dPDqCCD+PlaoaWfEI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706694298; a=rsa-sha256; cv=none; b=cTtuaVSeZ1vg6pNQDGEqQCQvZphSztGKxu9iC6swjl46Ipg8S33ua9yQYexbGKkqYrybNU JT1ozuDFmT8VX5MiT0662m7KPSRdfCjzCF4RxFaLhDEJQBMfrr08Mm2OPokLf0JbVfU6bX 1GFvle601lWc8TY6fX+QBKCu3sOtowU= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=imBBIY9y; spf=pass (imf22.hostedemail.com: domain of urezki@gmail.com designates 209.85.208.175 as permitted sender) smtp.mailfrom=urezki@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-lj1-f175.google.com with SMTP id 38308e7fff4ca-2d04c0b1cacso38001091fa.0 for ; Wed, 31 Jan 2024 01:44:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1706694296; x=1707299096; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:from:to:cc:subject:date:message-id:reply-to; bh=xqfllA/2YuYyJzutnMb4Kn7O4ZljTY6NQ9hi49SVfuU=; b=imBBIY9yrp3gli4NKaLwvXa4a2Se6+/+E/vM34WpVDZA4g235Wy4zqltzpbuGccaut zFXBCnvr0a7W+QGTd/z4YLdDQb06JVbaqJHSe0XYzAhMdUNEOFqNzcLjny3Yj+covVmr 1FN6D3/xmrJ+w4toEzHlQ42me2I71u7BM8AcdcNVqfZjWi8XCkpvwzTmwnptu/WSpB8e FBIGwAY4gSdsnjlMMKPdbzApQ+bS1hL6bw1Wrb0/URfcqtqPkJjxYFg+2JGR0XmnwUPv 2EzhHe0q9a/8Z4/4SH9XR7LLxsSyxRBTk3QpChRGqrw09v4eEzxUBeoR+d4SHkBC4h96 Y1MA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706694296; x=1707299096; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:date:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=xqfllA/2YuYyJzutnMb4Kn7O4ZljTY6NQ9hi49SVfuU=; b=Mfm2isk1HRHVF6lEFqKL/+2wftpedK73MuXisUumF1VNvgzXfMJz0BrEjkyKfCsBTM ZuaBcexBP2uzJML8nYkTPlcbvHe5kgsL1y1t2f3JY3x/azogAug9NvOi+yGs8zqU8+Nk SrWWOahIt3TRKH8hZc21OsILUcdXiLxqhiX19MF1b7hkLBfT5BgOVmYQ3tlqohrWBVWA p0JMXAIDvQ2BVLkDeRrn833UZqAw6L8VJsy+Oq3KSfJDfruAhUJIZRbdfQScnGgnjsKA 0y+7TGPUN5YmrftsQnDmJM3AhlLBSjSStUew5LiQELWoqrblMIP7PAzXJHFWEv3ewwTS RRZQ== X-Gm-Message-State: AOJu0YyzhhBgrtla4AXOOz8u7MyyXS695fafhtTW2F0/fmffl2TaYbO7 sCcHg2tjrxvc80ft+V0rApD9vobMb6FcSCkT/T8YziDEM04yrRDq X-Google-Smtp-Source: AGHT+IGqY3n86AQFMF9vP/idAYTWFZOWTzha8EHc3DX+Yo8i2h5ItRyBdF1G5YVvXvouBbe78z1EJw== X-Received: by 2002:a05:651c:204d:b0:2cd:a297:34 with SMTP id t13-20020a05651c204d00b002cda2970034mr676405ljo.3.1706694296009; Wed, 31 Jan 2024 01:44:56 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCXoe/C03lna4UHzlqle05g3e9qcmfQzYe9hWXeM/UYZO6fHuJJgSgV08W26TQS3HFOaTuALeyhryxNTmV5L9l/k0JtQWc7qP0zjoXnfyIosSZPowi+dEelDgFGc9l4XE8j9AEy0TXCg/k+KNfeKx0PTX3W2z/ofwC+bj0OGHzHPA79PGB5vinz2r8ds0Sbb1iz7W2TAhlAtUC9Uu0C7WoKFeWjoUJ5X19CYMqtrxaNEN66v8CRgFvNTyDQdO/4nxWzITpJ757EzO5+Tx3eWX/3UuqL0iBV1duAzemFb7SZwDVY3U8T1RH5TJl7yNg2fX987GAMV5knnTANE6zYCxfGv7Qpm1w== Received: from pc636 (host-90-233-198-33.mobileonline.telia.com. [90.233.198.33]) by smtp.gmail.com with ESMTPSA id t15-20020a2e9c4f000000b002d0495fa30fsm1141594ljj.47.2024.01.31.01.44.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Jan 2024 01:44:55 -0800 (PST) From: Uladzislau Rezki X-Google-Original-From: Uladzislau Rezki Date: Wed, 31 Jan 2024 10:44:53 +0100 To: Kees Cook Cc: Uladzislau Rezki , Lorenzo Stoakes , linux-hardening@vger.kernel.org, Andrew Morton , Christoph Hellwig , linux-mm@kvack.org, "Gustavo A. R. Silva" , Bill Wendling , Justin Stitt , linux-kernel@vger.kernel.org Subject: Re: [PATCH 33/82] mm/vmalloc: Refactor intentional wrap-around calculation Message-ID: References: <20240122235208.work.748-kees@kernel.org> <20240123002814.1396804-33-keescook@chromium.org> <24526e13-2df6-47ea-865f-b5a5594bc024@lucifer.local> <202401301356.4F87B727@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202401301356.4F87B727@keescook> X-Rspamd-Queue-Id: 23A5EC0017 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: t7of4rp3y9sdy4cme4d56dsozabqqu7b X-HE-Tag: 1706694297-66632 X-HE-Meta: U2FsdGVkX19nnxSORTsr/08Q23zuVNgahcIzm/l2pX4Vk1nf0XHlKi45dtOGOQV3OlMLMluCAeyCheu9/0tpjOsIlTxoJ3y20mJDjO46XXwh5TN0TaufCye3i1T2oglU3eLsfq6YC7GhM8TnPkB+mDzitK6F/7K6p6ZgzsOYeheKHhp4uKQBZqr9iCATX4ctMC4IHBr2gjFi7GASiGDC987bs2VlpR7Gge85jjF+2p4fIy5ZxoDbgTbPbfbmF6gv/fJ4MGILXYXbB5gYNYUd7mO4PFPLcMXF8ZERpxfOE9OWZ6WrNCnnNfmD3EibjrDTkj17Nt+11Eds6KXX7yc0uo6gdbqAud+S3o/vfN9brmzrFTLeLlK8U6na97/3Vk5NHCUab2R1ofgUYrCC8nkeo+Mv2FTy7qoV3Y/llGp1MILt7CHtZxwpTOT4Y8uhRN6ccTthPKkOi9ToKUjhbrfvyrPO7OAPgp18xSqSqdH8hRfD1iYmQhqev/EvMyPSugA6OoEVwZHpThQjHqsQaxDLIiqr1V6TZOXOORre0jylRMx7a7AxLXKPWaysXeJMoHwMx69xs+hXoS3TbW9IIiPtr5COm7SOgaBRMFdAmwIfaPD5A6O+pq7sKdthxxSulTHyPA7zzNLzD31FA0PPmdp7s92dXV3W9Nou3FF6L98ABRVzHF6ofAP16cR3smjSmh6YGcdEFzuS7If1LVw2FGFSejTWgCxnO36IuCR63XicwvhcXgC+6S/wCaqk03Z+Em/9wD46itRDlWTY7JP2vXcTr8nPY3OSr1KVfswy1ZfLExAZvxWflUiQ975+oY7tXm43I4SryTfcX3xrYDh3H6SgwVnVhL/PckQd4J1Xml7fbzMEovb5k442gJqlKqkC+K+DgBbBNSuK4smtfirJtxY9iX8QNQBIbyzarzJaxBwJP9O69VQqxp5hMdZKcrZShBmt2MnazQz+Re8qv2VMEN6 t91x6xKy RMwJjg6+fhYWCTRZVIM0ne4iDvwW5y5Fh/fF/wUk5PPChNikq1xFCkz8udmrama7mMcuN60Goz7HUlMU= 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 Tue, Jan 30, 2024 at 01:57:12PM -0800, Kees Cook wrote: > On Tue, Jan 30, 2024 at 08:54:00PM +0100, Uladzislau Rezki wrote: > > On Tue, Jan 30, 2024 at 06:55:57PM +0000, Lorenzo Stoakes wrote: > > > On Mon, Jan 22, 2024 at 04:27:08PM -0800, Kees Cook wrote: > > > > In an effort to separate intentional arithmetic wrap-around from > > > > unexpected wrap-around, we need to refactor places that depend on this > > > > kind of math. One of the most common code patterns of this is: > > > > > > > > VAR + value < VAR > > > > > > > > Notably, this is considered "undefined behavior" for signed and pointer > > > > types, which the kernel works around by using the -fno-strict-overflow > > > > option in the build[1] (which used to just be -fwrapv). Regardless, we > > > > want to get the kernel source to the position where we can meaningfully > > > > instrument arithmetic wrap-around conditions and catch them when they > > > > are unexpected, regardless of whether they are signed[2], unsigned[3], > > > > or pointer[4] types. > > > > > > > > Refactor open-coded unsigned wrap-around addition test to use > > > > check_add_overflow(), retaining the result for later usage (which removes > > > > the redundant open-coded addition). This paves the way to enabling the > > > > unsigned wrap-around sanitizer[2] in the future. > > > > > > > > Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] > > > > Link: https://github.com/KSPP/linux/issues/26 [2] > > > > Link: https://github.com/KSPP/linux/issues/27 [3] > > > > Link: https://github.com/KSPP/linux/issues/344 [4] > > > > Cc: Andrew Morton > > > > Cc: Uladzislau Rezki > > > > Cc: Christoph Hellwig > > > > Cc: Lorenzo Stoakes > > > > Cc: linux-mm@kvack.org > > > > Signed-off-by: Kees Cook > > > > --- > > > > mm/vmalloc.c | 5 +++-- > > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > > > > index d12a17fc0c17..7932ac99e9d3 100644 > > > > --- a/mm/vmalloc.c > > > > +++ b/mm/vmalloc.c > > > > @@ -1223,6 +1223,7 @@ is_within_this_va(struct vmap_area *va, unsigned long size, > > > > unsigned long align, unsigned long vstart) > > > > { > > > > unsigned long nva_start_addr; > > > > + unsigned long sum; > > > > > > > > if (va->va_start > vstart) > > > > nva_start_addr = ALIGN(va->va_start, align); > > > > @@ -1230,11 +1231,11 @@ is_within_this_va(struct vmap_area *va, unsigned long size, > > > > nva_start_addr = ALIGN(vstart, align); > > > > > > > > /* Can be overflowed due to big size or alignment. */ > > > > - if (nva_start_addr + size < nva_start_addr || > > > > + if (check_add_overflow(nva_start_addr, size, &sum) || > > > > nva_start_addr < vstart) > > > > return false; > > > > > > > > - return (nva_start_addr + size <= va->va_end); > > > > + return (sum <= va->va_end); > > > > } > > > > > > > > /* > > > > -- > > > > 2.34.1 > > > > > > > > > > Looks good to me, > > > > > > Reviewed-by: Lorenzo Stoakes > > > > > Same here. One small nit though. The "sum" variable is not something > > that it suits for. IMO, we should use a better name and replace it: > > > > "nva_offset"? > > Sure, I can use that. Other folks in other patches have suggested "end", > so maybe nva_end or nva_end_addr ? > nva_end_addr is probably the best fit. -- Uladzislau Rezki