[PATCH 0/3] irqchip/gic-v3: Memory-safety fixes on the probe paths

[PATCH 0/3] irqchip/gic-v3: Memory-safety fixes on the probe paths

[Fuad Tabba]

Hi folks,

Three probe-time bugs in the GICv3 drivers. The kernel keeps running
after each of them, so the memory involved is live rather than on the
way to a reset.

The first two free memory an enabled ITS still points at. Boot carries
on because its_init() discards a failed probe and gic_init_bases()
discards what its_init() returns. The third is a write past the end of
redist_regs[] for an enabled GICC with a zero GICR base.
fa2dabe57220e's commit message says such tables exist, and I haven't
seen one.

Based on Linux 7.3-rc4 (93f51579e7df2).

Cheers,
/fuad

Fuad Tabba (3):
  irqchip/gic-v3-its: Don't free a vPE table shared with another ITS
  irqchip/gic-v3-its: Don't free the tables of an enabled ITS
  irqchip/gic-v3: Don't register a redistributor that was never counted

 drivers/irqchip/irq-gic-v3-its.c | 16 +++++++++++-----
 drivers/irqchip/irq-gic-v3.c     |  6 ++++++
 2 files changed, 17 insertions(+), 5 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.39.5

[PATCH 1/3] irqchip/gic-v3-its: Don't free a vPE table shared with another ITS

[Fuad Tabba]

On GICv4.1, its_alloc_tables() copies a sibling's vPE table baser
rather than allocating one, but its_free_tables() frees it like the
rest. A probe failure on the copying ITS then frees pages the enabled
sibling still points GITS_BASER2 at, and the kernel comes up running
on that sibling.

Mark a shared table so its_free_tables() skips it.

Fixes: 5e5168461c22c ("irqchip/gic-v4.1: VPE table (aka GICR_VPROPBASER) allocation")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v3-its.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9807af235373..58e52095e6ea8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -87,6 +87,8 @@ struct its_baser {
 	u64		val;
 	u32		order;
 	u32		psz;
+	/* Inherited from a sibling ITS, not freed here */
+	bool		shared;
 };
 
 struct its_device;
@@ -2604,7 +2606,8 @@ static void its_free_tables(struct its_node *its)
 
 	for (i = 0; i < GITS_BASER_NR_REGS; i++) {
 		if (its->tables[i].base) {
-			its_free_pages(its->tables[i].base, its->tables[i].order);
+			if (!its->tables[i].shared)
+				its_free_pages(its->tables[i].base, its->tables[i].order);
 			its->tables[i].base = NULL;
 		}
 	}
@@ -2703,6 +2706,7 @@ static int its_alloc_tables(struct its_node *its)
 				WARN_ON(i != 2);
 				if ((sibling = find_sibling_its(its))) {
 					*baser = sibling->tables[2];
+					baser->shared = true;
 					its_write_baser(its, baser, baser->val);
 					continue;
 				}
-- 
2.39.5

[PATCH 2/3] irqchip/gic-v3-its: Don't free the tables of an enabled ITS

[Fuad Tabba]

its_probe_one() enables the ITS before its_init_domain(), the last step
that can fail, and the unwind from there frees the tables and the
command queue without disabling it. The kernel comes up with an enabled
ITS pointing at freed pages.

Enable the ITS last. Nothing reaches the new domain before then: this
runs from init_IRQ(), and the ITS joins its_nodes only after the enable.

Fixes: 4c21f3c26ecc2 ("irqchip: GICv3: ITS: DT probing and initialization")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v3-its.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 58e52095e6ea8..a5690721f0feb 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5332,16 +5332,18 @@ static int __init its_probe_one(struct its_node *its)
 	}
 
 	gits_write_cwriter(0, its->base + GITS_CWRITER);
+
+	err = its_init_domain(its);
+	if (err)
+		goto out_free_collection;
+
+	/* Enable last: the unwind frees memory an enabled ITS may access */
 	ctlr = readl_relaxed(its->base + GITS_CTLR);
 	ctlr |= GITS_CTLR_ENABLE;
 	if (is_v4(its))
 		ctlr |= GITS_CTLR_ImDe;
 	writel_relaxed(ctlr, its->base + GITS_CTLR);
 
-	err = its_init_domain(its);
-	if (err)
-		goto out_free_collection;
-
 	raw_spin_lock(&its_lock);
 	list_add(&its->entry, &its_nodes);
 	raw_spin_unlock(&its_lock);
-- 
2.39.5

[PATCH 3/3] irqchip/gic-v3: Don't register a redistributor that was never counted

[Fuad Tabba]

gic_acpi_match_gicc() counts only the enabled GICCs with a non-zero
gicr_base_address, and that count sizes redist_regs[], but
gic_acpi_parse_madt_gicc() registers every enabled one. For an enabled
GICC with a zero GICR base, gic_acpi_register_redist() therefore writes
a struct redist_region past the end of the array.

Commit fa2dabe57220e ("irqchip/gic-v3: Don't return errors from
gic_acpi_match_gicc()") removed the check that kept the two consistent;
its message says such entries are still caught by gic_populate_rdist(),
but that runs from gic_cpu_init(), after the write. Skip the entry
instead.

Fixes: fa2dabe57220e ("irqchip/gic-v3: Don't return errors from gic_acpi_match_gicc()")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/irqchip/irq-gic-v3.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 6e1fa5b247fc4..fb6a0570fb154 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -2344,6 +2344,12 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
 		return 0;
 	}
 
+	/* Not counted by gic_acpi_match_gicc(), so there is no slot for it */
+	if (!gicc->gicr_base_address) {
+		pr_warn(FW_BUG "GICC entry with ACPI UID %u has no GICR base address\n", gicc->uid);
+		return 0;
+	}
+
 	redist_base = ioremap(gicc->gicr_base_address, size);
 	if (!redist_base)
 		return -ENOMEM;
-- 
2.39.5

Re: [PATCH 3/3] irqchip/gic-v3: Don't register a redistributor that was never counted

[Marc Zyngier]

On Thu, 24 Sep 2026 17:15:14 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
> 
> gic_acpi_match_gicc() counts only the enabled GICCs with a non-zero
> gicr_base_address, and that count sizes redist_regs[], but
> gic_acpi_parse_madt_gicc() registers every enabled one. For an enabled
> GICC with a zero GICR base, gic_acpi_register_redist() therefore writes
> a struct redist_region past the end of the array.
> 
> Commit fa2dabe57220e ("irqchip/gic-v3: Don't return errors from
> gic_acpi_match_gicc()") removed the check that kept the two consistent;
> its message says such entries are still caught by gic_populate_rdist(),
> but that runs from gic_cpu_init(), after the write. Skip the entry
> instead.
> 
> Fixes: fa2dabe57220e ("irqchip/gic-v3: Don't return errors from gic_acpi_match_gicc()")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  drivers/irqchip/irq-gic-v3.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6e1fa5b247fc4..fb6a0570fb154 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2344,6 +2344,12 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
>  		return 0;
>  	}
>  
> +	/* Not counted by gic_acpi_match_gicc(), so there is no slot for it */
> +	if (!gicc->gicr_base_address) {
> +		pr_warn(FW_BUG "GICC entry with ACPI UID %u has no GICR base address\n", gicc->uid);
> +		return 0;
> +	}
> +

Shouldn't this also update the broken_rdist bitmap?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 3/3] irqchip/gic-v3: Don't register a redistributor that was never counted

[Fuad Tabba]

On Thu, 24 Sep 2026 18:29:28 +0100, Marc Zyngier <maz@kernel.org> wrote:

[...]

> Shouldn't this also update the broken_rdist bitmap?

Yes, it should. Without it gic_check_rdist() returns 0 and cpuhp
brings the CPU up. Sashiko raised the same [1], and I was in the
middle of replying with this:

        /* Not counted by gic_acpi_match_gicc(), so there is no slot for it */
        if (!gicc->gicr_base_address) {
                int cpu = get_cpu_for_acpi_id(gicc->uid);

                pr_warn(FW_BUG "GICC entry with ACPI UID %u has no
GICR base address, CPU kept offline\n",
                        gicc->uid);
                if (cpu >= 0)
                        cpumask_set_cpu(cpu, &broken_rdists);
                return 0;
        }

I'll fold it into a v2.

Cheers,
/fuad

[1] https://sashiko.dev/#/patchset/20260924161514.1112730-1-fuad.tabba%40linux.dev