Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
From:   Prarit Bhargava <prarit@redhat.com>
To:     linux-kernel@vger.kernel.org
Cc:     Prarit Bhargava <prarit@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Wei Liu <wei.liu@kernel.org>,
        Saurabh Sengar <ssengar@linux.microsoft.com>,
        Johan Hovold <johan+linaro@kernel.org>,
        Michael Kelley <mikelley@microsoft.com>,
        David Malcolm <dmalcolm@redhat.com>,
        David Arcari <darcari@redhat.com>,
        Don Zickus <dzickus@redhat.com>
Subject: [PATCH] x86/ioapic: io_apic fix null dereference check
Date:   Mon,  4 Dec 2023 13:02:26 -0500
Message-ID: <20231204180226.383745-1-prarit@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

The gcc plugin -fanalyzer [1] tries to detect various
patterns of incorrect behaviour.  The tool reports

arch/x86/kernel/apic/io_apic.c: In function ‘ioapic_destroy_irqdomain’:
arch/x86/kernel/apic/io_apic.c:2390:12: warning: check of ‘ioapics[idx].irqdomain’ for NULL after already dereferencing it [-Wanalyzer-deref-before-check]
    |
    | 2388 |         struct fwnode_handle *fn = ioapics[idx].irqdomain->fwnode;
    |      |                               ^~
    |      |                               |
    |      |                               (1) pointer ‘ioapics[idx].irqdomain’ is dereferenced here
    | 2389 |
    | 2390 |         if (ioapics[idx].irqdomain) {
    |      |            ~
    |      |            |
    |      |            (2) pointer ‘ioapics[idx].irqdomain’ is checked for NULL here but it was already dereferenced at (1)
    |

Fix the null dereference check in ioapic_destroy_irqdomain().

[1] https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/Static-Analyzer-Options.html

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: "Peter Zijlstra (Intel)" <peterz@infradead.org>
CC: Wei Liu <wei.liu@kernel.org>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Saurabh Sengar <ssengar@linux.microsoft.com>
CC: Johan Hovold <johan+linaro@kernel.org>
CC: Michael Kelley <mikelley@microsoft.com>
CC: David Malcolm <dmalcolm@redhat.com>
CC: David Arcari <darcari@redhat.com>
CC: Don Zickus <dzickus@redhat.com>
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 arch/x86/kernel/apic/io_apic.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 00da6cf6b07d..f6f19eee0339 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2381,14 +2381,14 @@ static int mp_irqdomain_create(int ioapic)
 static void ioapic_destroy_irqdomain(int idx)
 {
 	struct ioapic_domain_cfg *cfg = &ioapics[idx].irqdomain_cfg;
-	struct fwnode_handle *fn = ioapics[idx].irqdomain->fwnode;
 
-	if (ioapics[idx].irqdomain) {
-		irq_domain_remove(ioapics[idx].irqdomain);
-		if (!cfg->dev)
-			irq_domain_free_fwnode(fn);
-		ioapics[idx].irqdomain = NULL;
-	}
+	if (!ioapics[idx].irqdomain)
+		return;
+
+	irq_domain_remove(ioapics[idx].irqdomain);
+	if (!cfg->dev)
+		irq_domain_free_fwnode(ioapics[idx].irqdomain->fwnode);
+	ioapics[idx].irqdomain = NULL;
 }
 
 void __init setup_IO_APIC(void)
-- 
2.43.0