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 7AD8AC433EF for ; Sat, 12 Feb 2022 00:39:15 +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=6LDqybWpa7Z4Z3/JTZotKqpGdIMj/XCUfJfm7lLtTBM=; b=0iUrmEIiAD9HbM nxIdq8xW/KwmeQbmQQhXXzduvURFnpg6oawqcb8xpIaph0pO3lykJy04Dd0X39l0TZ02t5Xo2eOrS eFU0u7zrqg3cU4JsjgAsaS/wYFFE+o3KLln2GhxJhO3TL+E/+Ja6lTvlqtxo7Zsx1rKpjSFXga1QA hU+rQrW0zKD/B1HvNgLOPWx8ysXleeV28ZfUKyHTwOdyoU5EhnnYnXE8MOVfupWOV4jWEpdOL/ZvO E5QGdtX3duvlB/WHOkk/M30d4M0Dm3Z3qivKr4PS5XyupPSwcI/rM6P9bhjWMPucy8jlE/mNd6Ohu ZtCXkjkDAxjigpbtiuYw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIgQH-009C5f-Nf; Sat, 12 Feb 2022 00:37:57 +0000 Received: from mail-pj1-x1030.google.com ([2607:f8b0:4864:20::1030]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIgQD-009C4c-SK for linux-arm-kernel@lists.infradead.org; Sat, 12 Feb 2022 00:37:55 +0000 Received: by mail-pj1-x1030.google.com with SMTP id h14-20020a17090a130e00b001b88991a305so13410186pja.3 for ; Fri, 11 Feb 2022 16:37:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=7VKg0zjQwSALdbTIf5uSsnkD9DpXhrHD5gjop/xM2cY=; b=fVgW/+njctwyhjA00T4OTwKIyHC+ekd/gR2K1kuA66oeEQXIkPL1FXYiyWdAp1d0xn TbdfWzdca4NFtjxnNV2QMdmjjW4SM+aVVim50VDMFKC4dYF/vTsoAHkeValgFgcoC+S3 UO2HQ51kQVncYNuzO0ANIFL2p5kuevmP5FdQc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=7VKg0zjQwSALdbTIf5uSsnkD9DpXhrHD5gjop/xM2cY=; b=AabVHg3BNGUqNynxQuWLtJwYLIY0LRC3CKXBX8Op7dv/gZm7aLCVrctfRI0Q2T/ciH vvjiqdK4OaGR+vVji4L6ljJ5Ju57pudAn1Tbj7Td/kZ7V97bI/NhOmXgufYdAKDon40c i+3M26xUCPWtjzzSeo64MX7qVpLgUtLPlNp9Lr/wxy7sSCJgWvVrex7dYUIbjyT9zP7U 2MQnIDrP/JDGjlgG09Z558ecn429hrzXZtzeEb4UgIEDiXSl5hbbZRlOIIqaGzEZxjnF 0JX3SB2VJSX4UlZC3zVRd/xAUweXVWIuPdG1Xwdwb2YlQQoYb3QZUoXA0cfC0jEoNJ4T 3cPQ== X-Gm-Message-State: AOAM533IDnr8GJ0qPhedgSaMRry+/OCDb2Ei1XxGRZFIMp6aWKvQVmSG hHquvQxvDvG1he2kMxJmmMTKTQ== X-Google-Smtp-Source: ABdhPJy8rQL9XeZ5gO2kkhHcKJasWBPGH4H6gAKxFR64fdrgNu/QVz2ARaO/PRaZzGTt8kr6AuxU0Q== X-Received: by 2002:a17:90a:1a53:: with SMTP id 19mr2970043pjl.19.1644626270083; Fri, 11 Feb 2022 16:37:50 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id h26sm20567050pgm.72.2022.02.11.16.37.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Feb 2022 16:37:49 -0800 (PST) Date: Fri, 11 Feb 2022 16:37:48 -0800 From: Kees Cook To: Robin Murphy Cc: Ard Biesheuvel , Victor Erminpour , Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , "Rafael J. Wysocki" , Len Brown , ACPI Devel Maling List , Linux ARM , Linux Kernel Mailing List , trivial@kernel.org Subject: Re: [PATCH v2] ACPI/IORT: Fix GCC 12 warning Message-ID: <202202111623.A7881CC@keescook> References: <1644518851-16847-1-git-send-email-victor.erminpour@oracle.com> <202202101415.43750CEE@keescook> <3740c93e-9fde-f89f-9752-26ffff3ea274@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3740c93e-9fde-f89f-9752-26ffff3ea274@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220211_163753_957983_F982DB25 X-CRM114-Status: GOOD ( 39.77 ) 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 Fri, Feb 11, 2022 at 10:34:09AM +0000, Robin Murphy wrote: > Hi Kees, > > On 2022-02-10 23:47, Kees Cook wrote: > > On Thu, Feb 10, 2022 at 08:41:51PM +0100, Ard Biesheuvel wrote: > > > On Thu, 10 Feb 2022 at 19:48, Victor Erminpour > > > wrote: > > > > > > > > When building with automatic stack variable initialization, GCC 12 > > > > complains about variables defined outside of switch case statements. > > > > Move the variable into the case that uses it, which silences the warning: > > > > > > > > ./drivers/acpi/arm64/iort.c:1670:59: error: statement will never be executed [-Werror=switch-unreachable] > > > > 1670 | struct acpi_iort_named_component *ncomp; > > > > | ^~~~~ > > > > > > > > Signed-off-by: Victor Erminpour > > > > > > Please cc people that commented on your v1 when you send a v2. > > > > > > Still NAK, for the same reasons. > > > > Let me see if I can talk you out of this. ;) > > > > So, on the face of it, I agree with you: this is a compiler bug. However, > > it's still worth fixing. Just because it's valid C isn't a good enough > > reason to leave it as-is: we continue to minimize the subset of the > > C language the kernel uses if it helps us get the most out of existing > > compiler features. We've eliminated all kinds of other "valid C" from the > > kernel because it improves robustness, security, etc. This is certainly > > nothing like removing VLAs or implicit fallthrough, but given that this > > is, I think, the only remaining case of it (I removed all the others a > > while ago when I had the same issues with the GCC plugins), I'd like to > > get it fixed. > > It concerns me if minimising the subset of the C language that the kernel > uses is achieved by converting more of the kernel to a not-quite-C language > that is not formally specified anywhere, by prematurely adopting > newly-invented compiler options that clearly don't work properly (the GCC > warning message quoted above may as well be "error: giraffes are not purple" > for all the sense it makes.) Yeah, you're right. While it's a corner case, it's still important to get it fixed because it risks eroding people's good will for future work. What you (and Ard) bring up is just as important a roadblock as any of the other (many *sob*) roadblocks that have been overcome for its adoption. > From your security standpoint (and believe me, I really do have faith in > your expertise here), which of these sounds better: > > 1: Being able to audit code based on well-defined language semantics > > 2: Playing whack-a-mole as issues are discovered empirically. > > 3: Neither of the above, but a warm fuzzy feeling because hey someone said > "security" in a commit message. > > AFAICS you're effectively voting against #1, and the examples you've given > demonstrate that #2 is nowhere near reliable enough either, so where does > that leave us WRT actual secure and robust code in Linux? Well, I'm for #1, though perhaps with a more narrow view: some semantics are just weird/surprising. ;) Until I first encountered this warning a few years ago when working on GCC_PLUGIN_STRUCTLEAK_BYREF_ALL, I didn't even know putting declarations there was valid C. ;) Whack-a-mole is part of the work to make these kinds of treewide changes, but the hope is to find as much of it ahead of time as possible. And, no, I have no interest in security theater. (Not everything has equal levels of effectiveness, of course, but I don't think that's what you're saying.) > In fairness I'd have no objection to that patch if it came with a convincing > justification, but that is so far very much lacking. My aim here is not to > be a change-averse Luddite, but to try to find a compromise where I can > actually have some confidence in such changes being made. Let's not start > pretending that 3 100ml bottles of shampoo are somehow "safer" than a 300ml > bottle of shampoo... Sure. I think I am trying to take a pragmatic approach here, which is that gaining auto-var-init is a big deal (killing entire classes of vulnerabilities), but it comes with an annoying compiler bug (that we do get a warning about) for an uncommon code pattern that is easy to fix. So rather than delaying the defense until the sharp edge on the compiler gets fixed, I'd like to get the rest rolling while the edge is filed. -Kees -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel