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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D55B1C10F15 for ; Thu, 25 Apr 2024 17:39:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=IL20AIovSjY73FqJBRdWKz3hCfdLQyb8zdkCqlpHlRI=; b=q01D7vRr318vMV awtZLLIsZPNJBjg+Kf60IdiNInoYjvtzs6HY5HplGayRiXJtYonGoyA/swb1cWIN0q2jjbp8DVotM xONvMLVMF7gtdFJYLX9lxiP5tWCPLV7SlxnRGAypCipr3M4wRNt261/Kdp3Jf0GcNVGBaXzr48T8f ZWeG6DS56xWlCBZsBYXRJ4g+Zt8268hNi+hPrrHD3UOxLeDVyv6kcTYDURwp48yGmuKEpGOPDvbl8 vORHOiljb2ft4P5VY0wyk/Xi33+dn6kM9ZKZPaoFYKo/Z8TPypSAr2VMBPUztKvFsmQeVCZMsqFnb kqkL5s+WcC52JNbVKCDQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s034F-00000009c8F-0gNw; Thu, 25 Apr 2024 17:39:31 +0000 Received: from mail-pg1-x52c.google.com ([2607:f8b0:4864:20::52c]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s034B-00000009c5Z-40kP for linux-arm-kernel@lists.infradead.org; Thu, 25 Apr 2024 17:39:29 +0000 Received: by mail-pg1-x52c.google.com with SMTP id 41be03b00d2f7-5dbcfa0eb5dso1056621a12.3 for ; Thu, 25 Apr 2024 10:39:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1714066762; x=1714671562; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=xGRp4J8KaAZoR1Iq7v17JFw6BNoCDdtd2vscqG11gNs=; b=Enh1wivGkScfM5BwEUZpBjkHOQcIND35KOiLn47gQrxFpu78/Geci3ujMmlQcNd0uu PQQADDOH/W7XyqKytBa84F8sXPASUN8iAhSqibwXN/YKqMWLyx5uVhUJ/KMR46U1DigV ro8p7R45EM8BSO0rux/tvnpSC+4oj+MYzYDyM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714066762; x=1714671562; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=xGRp4J8KaAZoR1Iq7v17JFw6BNoCDdtd2vscqG11gNs=; b=sLQkqq+0RXAgb9a6E+ykMb3aWOjUWxpcEKuBprO/pAI/RmhumvPrdUNvo4co0ncdgF vd58U2S6+z4S5PmrIOyAw1UzKIASWGD94AfHDQLeiN8UVm6f6hIGXeHZmqFwaSvETUQj 6wbrpCS+LzMD4zEp2UISkM74DP7oA9UvhnCheg9ME1MI8UZjKLev7jmLFn5a4e8irpEW o02MQLXyi+tZ5TGHvE5sYd6IkCRF2mlboKdZu/ipo/XrrixbOqed+gFdzIrY7ymPyGtC vud44DQJxx9pMf6xaDW1Apf2OG3yMq4vlRHtiEB7IY6sOQV5AI2thynyhskR/ZhWJoOP Wydw== X-Forwarded-Encrypted: i=1; AJvYcCUMGweAQHx0yE9H1OayuphyEKFDZl4xt00QXy86eINFQLpzzxnX5LNAx6F2EH5+f0/1KRMryWA6whjTcSMPMRkYJd/CyCQVG/NfRcmPU8xV43IEcLY= X-Gm-Message-State: AOJu0YzjnhHd/Khvbt/F7fLwNQiolkrFfcIzdylnwmi0cu8bnSVP8GLc a+1GgWVZ3Hjxnn17SqWgBeMstvMImRxbZUX26KHum9bCCSSZeCUW8b2nWGCECw== X-Google-Smtp-Source: AGHT+IHo6ruBh+6PnC5lx4dAR90eGtu9va6/DL9++gNNA1zkeY3/L32beujc9vp0VhFbPkSwUX9WUg== X-Received: by 2002:a05:6a20:5b12:b0:1ad:3d93:b71e with SMTP id kl18-20020a056a205b1200b001ad3d93b71emr347251pzb.59.1714066761597; Thu, 25 Apr 2024 10:39:21 -0700 (PDT) Received: from www.outflux.net ([198.0.35.241]) by smtp.gmail.com with ESMTPSA id ck16-20020a17090afe1000b002a2f6da006csm13262979pjb.52.2024.04.25.10.39.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Apr 2024 10:39:21 -0700 (PDT) Date: Thu, 25 Apr 2024 10:39:20 -0700 From: Kees Cook To: Peter Zijlstra Cc: Mark Rutland , Will Deacon , Boqun Feng , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Jakub Kicinski , Catalin Marinas , Arnd Bergmann , Andrew Morton , "David S. Miller" , David Ahern , Eric Dumazet , Paolo Abeni , "Paul E. McKenney" , Uros Bizjak , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org, netdev@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH 1/4] locking/atomic/x86: Silence intentional wrapping addition Message-ID: <202404251019.2DF0A48@keescook> References: <20240424191225.work.780-kees@kernel.org> <20240424191740.3088894-1-keescook@chromium.org> <20240424224141.GX40213@noisy.programming.kicks-ass.net> <202404241542.6AFC3042C1@keescook> <20240424225436.GY40213@noisy.programming.kicks-ass.net> <202404241602.276D4ADA@keescook> <20240425091752.GA21980@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240425091752.GA21980@noisy.programming.kicks-ass.net> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240425_103928_010059_1F75A745 X-CRM114-Status: GOOD ( 41.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 25, 2024 at 11:17:52AM +0200, Peter Zijlstra wrote: > On Wed, Apr 24, 2024 at 04:20:20PM -0700, Kees Cook wrote: > > > > This is arse-about-face. Signed stuff wraps per -fno-strict-overflow. > > > We've been writing code for years under that assumption. > > > > Right, which is why this is going to take time to roll out. :) What we > > were really doing with -fno-strict-overflow was getting rid of undefined > > behavior. That was really really horrible; we don't need the compiler > > hallucinating. > > Right, but that then got us well defined semantics for signed overflow. Yes, and this gets us to the next step: disambiguation for general users. It's good that we have a well-defined overflow resolution strategy, but our decades of persistent wrap-around flaws in the kernel show that many devs (even experienced ones) produce code with unexpected and unwanted (to the logic of the code) wrap-around. So we have to find a way to distinguish wrapping and non-wrapping operations or types up front and in a clear way. > > > > You want to mark the non-wrapping case. > > > > What we want is lack of ambiguity. Having done these kinds of things in > > the kernel for a while now, I have strong evidence that we get much better > > results with the "fail safe" approach, but start by making it non-fatal. > > That way we get full coverage, but we don't melt the world for anyone > > that doesn't want it, and we can shake things out over a few years. For > > example, it has worked well for CONFIG_FORTIFY, CONFIG_UBSAN_BOUNDS, > > KCFI, etc. > > The non-fatal argument doesn't have bearing on the mark warp or mark > non-wrap argument though. This gets at the strategy of refactoring our code to gain our unambiguous coverage. Since we can't sanely have a flag-day, we have to go piecemeal, and there will continue to be places where the coverage was missed, and so we want to progress through marking wrapping cases without BUGing the kernel. (We don't care about catching non-wrapping -- the exceptional condition is hitting an overflow.) > > The riskier condition is having something wrap when it wasn't expected > > (e.g. allocations, pointer offsets, etc), so we start by defining our > > regular types as non-wrapping, and annotate the wrapping types (or > > specific calculations or functions). > > But but most of those you mention are unsigned. Are you saying you're > making all unsigned variables non-wrap by default too? That's bloody > insane. We have a mix (and a regular confusion even in core code) where "int" gets passed around even though at one end or another of a call chain it's actually u32 or u16 or whatever. Regardless, yes, the next step after signed overflow mitigation would be unsigned overflow mitigation, and as you suggest, it's much more tricky. > > For signed types in particular, wrapping is overwhelmingly the > > uncommon case, so from a purely "how much annotations is needed" > > perspective, marking wrapping is also easiest. Yes, there are cases of > > expected wrapping, but we'll track them all down and get them marked > > unambiguously. > > But I am confused now, because above you seem to imply you're making > unsigned non-wrap too, and there wrapping is *far* more common, and I > must say I hate this wrapping_add() thing with a passion. Yes, most people are not a fan of the wrapping_*() helpers, which is why I'm trying to get a typedef attribute created. But again, to gain the "fail safe by default" coverage, we have to start with the assumption that the default is non-wrapping, and mark those that aren't. (Otherwise we're not actually catching unexpected cases.) And no, it's not going to be over-night. It's taken almost 5 years to disambiguate array bounds and we're still not done. :) > > One thing on the short list is atomics, so here we are. :) > > Well, there are wrapping and non-wrapping users of atomic. If only C had > generics etc.. (and yeah, _Generic doesn't really count). Non-wrapping users of atomics should be using refcount_t, which is our non-wrapping atomic type. But regardless, atomics are internally wrapping, yes? Anyway, I suspect this whole plan needs wider discussion. I will write up a more complete RFC that covers my plans, including the rationale for why we should adopt this in a certain way. (These kinds of strategic RFCs don't usually get much traction since our development style is much more "show the patches", so that's why I have been just sending patches. But since it's a pretty big topic, I'll give it a shot...) -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel