1. Patchew
  2. linux
  3. View series

[PATCH] irqchip/gic-v3: Work around insecure GIC integrations

Marc Zyngier posted 1 patch 1 year ago 
drivers/irqchip/irq-gic-v3.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
[PATCH] irqchip/gic-v3: Work around insecure GIC integrations
Posted by Marc Zyngier 1 year ago 
It appears that the relatively popular RK3399 SoC has been put together
using a large amount of illicit substances, as experiments reveal
that its integration of GIC500 exposes the *secure* programming
interface to non-secure.

This has some pretty bad effects on the way priorities are handled,
and results in a dead machine if booting with pseudo-NMI enabled
(irqchip.gicv3_pseudo_nmi=1) if the kernel contains 18fdb6348c480
("arm64: irqchip/gic-v3: Select priorities at boot time"), which
relies on the priorities being programmed using the NS view.

Let's restore some sanity by going one step further and disable
security altogether in this case. This is not any worse, and
puts us in a mode where priorities actually make some sense.

Huge thanks to Mark Kettenis who initially identified this issue
on OpenBSD, and to Chen-Yu Tsai who reported the problem in
Linux.

Fixes: 18fdb6348c480 ("arm64: irqchip/gic-v3: Select priorities at boot time")
Reported-by: Mark Kettenis <mark.kettenis@xs4all.nl>
Reported-by: Chen-Yu Tsai <wenst@chromium.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/irqchip/irq-gic-v3.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 34db379d066a5..79d8cc80693c3 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -161,7 +161,22 @@ static bool cpus_have_group0 __ro_after_init;
 
 static void __init gic_prio_init(void)
 {
-	cpus_have_security_disabled = gic_dist_security_disabled();
+	bool ds;
+
+	ds = gic_dist_security_disabled();
+	if (!ds) {
+		u32 val;
+
+		val = readl_relaxed(gic_data.dist_base + GICD_CTLR);
+		val |= GICD_CTLR_DS;
+		writel_relaxed(val, gic_data.dist_base + GICD_CTLR);
+
+		ds = gic_dist_security_disabled();
+		if (ds)
+			pr_warn("Broken GIC integration, security disabled");
+	}
+
+	cpus_have_security_disabled = ds;
 	cpus_have_group0 = gic_has_group0();
 
 	/*
-- 
2.39.2
Re: [PATCH] irqchip/gic-v3: Work around insecure GIC integrations
Posted by Chen-Yu Tsai 1 year ago 
On Fri, Dec 13, 2024 at 10:34 PM Marc Zyngier <maz@kernel.org> wrote:
>
> It appears that the relatively popular RK3399 SoC has been put together
> using a large amount of illicit substances, as experiments reveal
> that its integration of GIC500 exposes the *secure* programming
> interface to non-secure.
>
> This has some pretty bad effects on the way priorities are handled,
> and results in a dead machine if booting with pseudo-NMI enabled
> (irqchip.gicv3_pseudo_nmi=1) if the kernel contains 18fdb6348c480
> ("arm64: irqchip/gic-v3: Select priorities at boot time"), which
> relies on the priorities being programmed using the NS view.
>
> Let's restore some sanity by going one step further and disable
> security altogether in this case. This is not any worse, and
> puts us in a mode where priorities actually make some sense.
>
> Huge thanks to Mark Kettenis who initially identified this issue
> on OpenBSD, and to Chen-Yu Tsai who reported the problem in
> Linux.
>
> Fixes: 18fdb6348c480 ("arm64: irqchip/gic-v3: Select priorities at boot time")
> Reported-by: Mark Kettenis <mark.kettenis@xs4all.nl>
> Reported-by: Chen-Yu Tsai <wenst@chromium.org>

Should be

Reported-by: Chen-Yu Tsai <wens@csie.org>

(I know it's confusing, I even mix up inboxes at work.)

> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org

Tested-by: Chen-Yu Tsai <wens@csie.org>

My RK3399 boots normally with pseudo NMI enabled with this patch now.
Also tried NMI backtraces through sysrq, though I'm not sure that
always goes through the pseudo NMI path?

> ---
>  drivers/irqchip/irq-gic-v3.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 34db379d066a5..79d8cc80693c3 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -161,7 +161,22 @@ static bool cpus_have_group0 __ro_after_init;
>
>  static void __init gic_prio_init(void)
>  {
> -       cpus_have_security_disabled = gic_dist_security_disabled();
> +       bool ds;
> +
> +       ds = gic_dist_security_disabled();
> +       if (!ds) {
> +               u32 val;
> +
> +               val = readl_relaxed(gic_data.dist_base + GICD_CTLR);
> +               val |= GICD_CTLR_DS;
> +               writel_relaxed(val, gic_data.dist_base + GICD_CTLR);
> +
> +               ds = gic_dist_security_disabled();
> +               if (ds)
> +                       pr_warn("Broken GIC integration, security disabled");
> +       }
> +
> +       cpus_have_security_disabled = ds;
>         cpus_have_group0 = gic_has_group0();
>
>         /*
> --
> 2.39.2
>
>
Re: [PATCH] irqchip/gic-v3: Work around insecure GIC integrations
Posted by Marc Zyngier 1 year ago 
On Fri, 13 Dec 2024 15:10:11 +0000,
Chen-Yu Tsai <wens@kernel.org> wrote:
> 
> On Fri, Dec 13, 2024 at 10:34 PM Marc Zyngier <maz@kernel.org> wrote:
> >
> > It appears that the relatively popular RK3399 SoC has been put together
> > using a large amount of illicit substances, as experiments reveal
> > that its integration of GIC500 exposes the *secure* programming
> > interface to non-secure.
> >
> > This has some pretty bad effects on the way priorities are handled,
> > and results in a dead machine if booting with pseudo-NMI enabled
> > (irqchip.gicv3_pseudo_nmi=1) if the kernel contains 18fdb6348c480
> > ("arm64: irqchip/gic-v3: Select priorities at boot time"), which
> > relies on the priorities being programmed using the NS view.
> >
> > Let's restore some sanity by going one step further and disable
> > security altogether in this case. This is not any worse, and
> > puts us in a mode where priorities actually make some sense.
> >
> > Huge thanks to Mark Kettenis who initially identified this issue
> > on OpenBSD, and to Chen-Yu Tsai who reported the problem in
> > Linux.
> >
> > Fixes: 18fdb6348c480 ("arm64: irqchip/gic-v3: Select priorities at boot time")
> > Reported-by: Mark Kettenis <mark.kettenis@xs4all.nl>
> > Reported-by: Chen-Yu Tsai <wenst@chromium.org>
> 
> Should be
> 
> Reported-by: Chen-Yu Tsai <wens@csie.org>
> 
> (I know it's confusing, I even mix up inboxes at work.)

Ah, fair enough. Given that the report was on IRC, I had just used the
first address git log gave me.

> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Cc: stable@vger.kernel.org
> 
> Tested-by: Chen-Yu Tsai <wens@csie.org>
> 
> My RK3399 boots normally with pseudo NMI enabled with this patch now.
> Also tried NMI backtraces through sysrq, though I'm not sure that
> always goes through the pseudo NMI path?

Should do, according to arch/arm64/kernel/smp::ipi_setup().

Thanks for having tested it.

	M.

-- 
Without deviation from the norm, progress is not possible.
[tip: irq/urgent] irqchip/gic-v3: Work around insecure GIC integrations
Posted by tip-bot2 for Marc Zyngier 1 year ago 
The following commit has been merged into the irq/urgent branch of tip:

Commit-ID:     773c05f417fa14e1ac94776619e9c978ec001f0b
Gitweb:        https://git.kernel.org/tip/773c05f417fa14e1ac94776619e9c978ec001f0b
Author:        Marc Zyngier <maz@kernel.org>
AuthorDate:    Fri, 13 Dec 2024 14:10:37 
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 13 Dec 2024 18:15:29 +01:00

irqchip/gic-v3: Work around insecure GIC integrations

It appears that the relatively popular RK3399 SoC has been put together
using a large amount of illicit substances, as experiments reveal that its
integration of GIC500 exposes the *secure* programming interface to
non-secure.

This has some pretty bad effects on the way priorities are handled, and
results in a dead machine if booting with pseudo-NMI enabled
(irqchip.gicv3_pseudo_nmi=1) if the kernel contains 18fdb6348c480 ("arm64:
irqchip/gic-v3: Select priorities at boot time"), which relies on the
priorities being programmed using the NS view.

Let's restore some sanity by going one step further and disable security
altogether in this case. This is not any worse, and puts us in a mode where
priorities actually make some sense.

Huge thanks to Mark Kettenis who initially identified this issue on
OpenBSD, and to Chen-Yu Tsai who reported the problem in Linux.

Fixes: 18fdb6348c480 ("arm64: irqchip/gic-v3: Select priorities at boot time")
Reported-by: Mark Kettenis <mark.kettenis@xs4all.nl>
Reported-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Chen-Yu Tsai <wens@csie.org>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/20241213141037.3995049-1-maz@kernel.org

---
 drivers/irqchip/irq-gic-v3.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 34db379..79d8cc8 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -161,7 +161,22 @@ static bool cpus_have_group0 __ro_after_init;
 
 static void __init gic_prio_init(void)
 {
-	cpus_have_security_disabled = gic_dist_security_disabled();
+	bool ds;
+
+	ds = gic_dist_security_disabled();
+	if (!ds) {
+		u32 val;
+
+		val = readl_relaxed(gic_data.dist_base + GICD_CTLR);
+		val |= GICD_CTLR_DS;
+		writel_relaxed(val, gic_data.dist_base + GICD_CTLR);
+
+		ds = gic_dist_security_disabled();
+		if (ds)
+			pr_warn("Broken GIC integration, security disabled");
+	}
+
+	cpus_have_security_disabled = ds;
 	cpus_have_group0 = gic_has_group0();
 
 	/*

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.