linux-kernel - [RFC PATCH] irqchip/gic/gic-v3: prevent GSI to SGI translations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon,  4 Apr 2022 12:08:42 +0100
From:   Andre Przywara <andre.przywara@....com>
To:     Marc Zyngier <maz@...nel.org>, Thomas Gleixner <tglx@...utronix.de>
Cc:     "Rafael J . Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Hanjun Guo <guohanjun@...wei.com>,
        Sudeep Holla <sudeep.holla@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: [RFC PATCH] irqchip/gic/gic-v3: prevent GSI to SGI translations

At the moment the GIC IRQ domain translation routine happily converts
ACPI table GSI numbers below 16 to GIC SGIs (Software Generated
Interrupts aka IPIs). On the Devicetree side we explicitly forbid this
translation, actually the function will never return HWIRQs below 16 when
using a DT based domain translation.

We expect SGIs to be handled in the first part of the function, and any
further occurrence should be treated as a firmware bug, so add a check
and print to report this explicitly and avoid lengthy debug sessions.

Fixes: 64b499d8df40 ("irqchip/gic-v3: Configure SGIs as standard interrupts")
Signed-off-by: Andre Przywara <andre.przywara@....com>
---
Hi,

marking this as an RFC because:
- I don't know if we really can forbid IPIs in ACPI tables. We certainly
  pushed back against this multiple times on the DT side.
- I don't know if this is the right place to filter this out.

This was triggered by an SSDT table wrongly containing an interrupt
resource entry of "0", for a debug UART without an interrupt line
connected [1] (about to be fixed). This overwrote the IPI0 trigger method
to "level", which prevented SGI0 to be enabled again *after* a CPU
offline/online cycle.
It required some debugging to find this firmware problem, so I am
proposing an explicit error message, and to actually deny registering
this interrupt.

[1] https://github.com/tianocore/edk2-platforms/blob/master/Platform/ARM/Morello/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c#L150-L157

Cheers,
Andre

 drivers/irqchip/irq-gic-v3.c | 6 ++++++
 drivers/irqchip/irq-gic.c    | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 0efe1a9a9f3b..c946ef3067d2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1466,6 +1466,12 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 		if(fwspec->param_count != 2)
 			return -EINVAL;
 
+		if (fwspec->param[0] < 16) {
+			pr_err(FW_BUG "Illegal GSI%d translation request\n",
+			       fwspec->param[0]);
+			return -EINVAL;
+		}
+
 		*hwirq = fwspec->param[0];
 		*type = fwspec->param[1];
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 58ba835bee1f..09c710ecc387 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1123,6 +1123,12 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 		if(fwspec->param_count != 2)
 			return -EINVAL;
 
+		if (fwspec->param[0] < 16) {
+			pr_err(FW_BUG "Illegal GSI%d translation request\n",
+			       fwspec->param[0]);
+			return -EINVAL;
+		}
+
 		*hwirq = fwspec->param[0];
 		*type = fwspec->param[1];
 
-- 
2.25.1