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 5E9C0C433FE for ; Thu, 10 Feb 2022 23:48:50 +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=QIFhdeGv8McpTYbUAF0ZgX71mdLSVqH8STzBpmVu+9k=; b=t14bDbFa3FR+o+ TXdcokNYtzUsrwM1chyXctK73NSzgemfhzPn+efrVyKxxgbswfAIVqYRTMJTzyBbvxdGBCWwhINOz lv8XuG//rdWmsd9YPpt8FM9Z+mGWh+xtDNe+SnHWKD5HDJDQKf+f+bQeupmgMW6LTNFyN/0+d+u8m cHhJGUJ8moKeLK9sKciptFowUx9kZxSE+HXvmWi7izgKfxBNp4Q3wPjHYFCs3qAow8IU+xGvtt8iB Tolkt1qgvVlpqzclOKQlKSc1KNxt4fDnH+0ejKSRJP+S4HpIlAy/Dl4aFJYjf8V9DR18xhI7ytqMl ZlKiy6g3iDIotng2hH7A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIJ9f-005E6a-UZ; Thu, 10 Feb 2022 23:47:16 +0000 Received: from mail-pf1-x433.google.com ([2607:f8b0:4864:20::433]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIJ9b-005E6B-3G for linux-arm-kernel@lists.infradead.org; Thu, 10 Feb 2022 23:47:14 +0000 Received: by mail-pf1-x433.google.com with SMTP id i21so11637214pfd.13 for ; Thu, 10 Feb 2022 15:47:10 -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=mHRsq6Kn87GMOD21+jYbugw/48vB/ViEdc86yH1U1RM=; b=cE/HfRBcMAiI7aHiAvjjmt/SqsHa2f/rPVtV36FfX+RNibf+yKHCwYxGDTUtUIoQTN n+ZC03sN7lJOgFIjtcfdqZc2niTOFjCQnJAiJcOp6p/xKpuK41BgjFbGKDbl7AUWmPk0 ur5mEAX2mLkDvRd+aBSDYM0PrWWRnuHFIpzh0= 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=mHRsq6Kn87GMOD21+jYbugw/48vB/ViEdc86yH1U1RM=; b=qPyvRzOl2UgG0RhlNEacDJxEYoRjwZfDAE726a3ejjarUAbNFKwQqtUqjqXioCdW3Z QIvK1ZsLDJbsjZuIOBvtpn47O9c3I70r5DWk24SjUD755v7BXoy1YGa2B0aprvDcyLSU NAm4PCSEz6NpKhk8egZ4A8DUAanrTVn8/YvqusnY9jNzbGrqpxFw+iOYoFg4WQaMHCVB cEX1HHPSWnzAZfQ+VS+mJ/UerQKWc1ABVi18fWTmuI1g2dDfmutBgrZ7T6oHlBUOiy3E PAKK48qHUsjJmpZ9vvUsYdbo5XCVoWj/lyvfCltf1SQ7PiIV3b1EA45XGBZ8M5izoFGK 3NnA== X-Gm-Message-State: AOAM531WdI+3hLEocfj3EOU2VexNL+kPOAMfy7IQUJ9omC5ORBs2BAEv 0cPHQ3+NFyw3Qs7i270VFo0JLQ== X-Google-Smtp-Source: ABdhPJyGc6kYqWRf+VlsVeyHDKoWC+0J4l3+nlcgViM0mwZ71ZFfynFkb2DC+466DdIh8ebqBFav6w== X-Received: by 2002:a63:7019:: with SMTP id l25mr1843865pgc.251.1644536829788; Thu, 10 Feb 2022 15:47:09 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id 69sm17784092pgc.61.2022.02.10.15.47.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Feb 2022 15:47:09 -0800 (PST) Date: Thu, 10 Feb 2022 15:47:08 -0800 From: Kees Cook To: Ard Biesheuvel Cc: Victor Erminpour , Robin Murphy , 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: <202202101415.43750CEE@keescook> References: <1644518851-16847-1-git-send-email-victor.erminpour@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220210_154711_176108_FF1A7BCC X-CRM114-Status: GOOD ( 35.94 ) 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, 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. And I should point out that Clang suffers[1] from the same problem (the variables will be missed for auto-initialization), but actually has a worse behavior: it does not even warn about it. And note that the problem isn't limited to -ftrivial-auto-var-init. This code pattern seems to also hide the variables from similar instrumentation like KASan, etc. (Which is similarly silent like above.) In both compilers, it seems fixing this is not "easy", and given its corner-case nature and ease of being worked around in the kernel source, it isn't being highly prioritized. But since I both don't want these blinds spots with Clang (and GCC) var-init, and I don't want these warnings to suddenly appear once GCC 12 _does_ get released, so I'd like to get this case fixed as well. All that said, I think this patch could be improved. I'd recommend, instead, just simply: diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index f2f8f05662de..9e765d30da82 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1671,13 +1671,14 @@ phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void) end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length); for (i = 0; i < iort->node_count; i++) { + struct acpi_iort_named_component *ncomp; + struct acpi_iort_root_complex *rc; + phys_addr_t local_limit; + if (node >= end) break; switch (node->type) { - struct acpi_iort_named_component *ncomp; - struct acpi_iort_root_complex *rc; - phys_addr_t local_limit; case ACPI_IORT_NODE_NAMED_COMPONENT: ncomp = (struct acpi_iort_named_component *)node->node_data; This results in no change in binary instruction output (when there is no auto-init). -Kees [1] https://github.com/llvm/llvm-project/issues/44261 > > > > --- > > drivers/acpi/arm64/iort.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > > index 3b23fb775ac4..65395f0decf9 100644 > > --- a/drivers/acpi/arm64/iort.c > > +++ b/drivers/acpi/arm64/iort.c > > @@ -1645,7 +1645,7 @@ void __init acpi_iort_init(void) > > */ > > phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void) > > { > > - phys_addr_t limit = PHYS_ADDR_MAX; > > + phys_addr_t local_limit, limit = PHYS_ADDR_MAX; > > struct acpi_iort_node *node, *end; > > struct acpi_table_iort *iort; > > acpi_status status; > > @@ -1667,17 +1667,16 @@ phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void) > > break; > > > > switch (node->type) { > > + case ACPI_IORT_NODE_NAMED_COMPONENT: { > > struct acpi_iort_named_component *ncomp; > > - struct acpi_iort_root_complex *rc; > > - phys_addr_t local_limit; > > - > > - case ACPI_IORT_NODE_NAMED_COMPONENT: > > ncomp = (struct acpi_iort_named_component *)node->node_data; > > local_limit = DMA_BIT_MASK(ncomp->memory_address_limit); > > limit = min_not_zero(limit, local_limit); > > break; > > > > - case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: > > + } > > + case ACPI_IORT_NODE_PCI_ROOT_COMPLEX: { > > + struct acpi_iort_root_complex *rc; > > if (node->revision < 1) > > break; > > > > @@ -1686,6 +1685,7 @@ phys_addr_t __init acpi_iort_dma_get_max_cpu_address(void) > > limit = min_not_zero(limit, local_limit); > > break; > > } > > + } > > node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length); > > } > > acpi_put_table(&iort->header); > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel