Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] irqchip: validate ACPI MADT entries before use
@ 2026-10-16 14:13 Jiale Yao
  2026-10-16 14:13 ` [PATCH 2/6] irqchip/gic-v3-its: validate MADT ITS entries Jiale Yao
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jiale Yao @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Marc Zyngier, Thomas Gleixner,
	Radu Rendec, Lorenzo Pieralisi, Huacai Chen, WANG Xuerui,
	Jiaxun Yang, linux-acpi, linux-kernel, linux-arm-kernel,
	loongarch, linux-mips
  Cc: Jiale Yao

IRQ chip ACPI probing receives MADT records supplied by firmware.  The
common probe path does not currently verify that the complete declared
entry is contained in the table, and several callbacks access
fixed-size records without checking their minimum size.  Malformed
entries can therefore cause out-of-bounds reads during early boot.

Add the common declared-range check first, then validate the concrete
ITS, GIC, LIO-PIC, and BIO-PIC records in their respective callbacks.
Each patch remains independently buildable and limits its changes to one
parser implementation.

Jiale Yao (6):
  ACPI: irqchip: validate MADT entry bounds before probing
  irqchip/gic-v3-its: validate MADT ITS entries
  irqchip/gic-v3: validate MADT records before use
  irqchip/gic-v5-its: validate MADT translate frames
  irqchip/loongarch-cpu: validate MADT LIO-PIC entries
  irqchip/loongson-eiointc: validate MADT BIO-PIC entries

 drivers/acpi/scan.c                    |  7 +++++++
 drivers/irqchip/irq-gic-v3-its.c       |  3 +++
 drivers/irqchip/irq-gic-v3.c           | 18 ++++++++++++++++++
 drivers/irqchip/irq-gic-v5-its.c       |  3 +++
 drivers/irqchip/irq-loongarch-cpu.c    |  3 +++
 drivers/irqchip/irq-loongson-eiointc.c | 10 ++++++++--
 6 files changed, 42 insertions(+), 2 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/6] irqchip/gic-v3-its: validate MADT ITS entries
  2026-10-16 14:13 [PATCH 0/6] irqchip: validate ACPI MADT entries before use Jiale Yao
@ 2026-10-16 14:13 ` Jiale Yao
  2026-10-16 14:13 ` [PATCH 3/6] irqchip/gic-v3: validate MADT records before use Jiale Yao
  2026-10-16 14:13 ` [PATCH 4/6] irqchip/gic-v5-its: validate MADT translate frames Jiale Yao
  2 siblings, 0 replies; 6+ messages in thread
From: Jiale Yao @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Radu Rendec, linux-arm-kernel,
	linux-kernel
  Cc: Jiale Yao

The MADT ITS callback reads the translator base address and ID without
checking that firmware supplied a complete generic translator record. A
truncated entry can therefore cause out-of-bounds reads.

Reject incomplete ITS entries before accessing their typed fields.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9807af23537..485a790e4856 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5725,6 +5725,9 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
 	int err;
 
 	its_entry = (struct acpi_madt_generic_translator *)header;
+	if (BAD_MADT_ENTRY(its_entry, end))
+		return -EINVAL;
+
 	memset(&res, 0, sizeof(res));
 	res.start = its_entry->base_address;
 	res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/6] irqchip/gic-v3: validate MADT records before use
  2026-10-16 14:13 [PATCH 0/6] irqchip: validate ACPI MADT entries before use Jiale Yao
  2026-10-16 14:13 ` [PATCH 2/6] irqchip/gic-v3-its: validate MADT ITS entries Jiale Yao
@ 2026-10-16 14:13 ` Jiale Yao
  2026-10-16 14:36   ` Marc Zyngier
  2026-10-16 14:13 ` [PATCH 4/6] irqchip/gic-v5-its: validate MADT translate frames Jiale Yao
  2 siblings, 1 reply; 6+ messages in thread
From: Jiale Yao @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Radu Rendec, linux-arm-kernel,
	linux-kernel
  Cc: Jiale Yao

The GICv3 ACPI paths cast generic MADT headers to distributor,
redistributor, and CPU interface records before confirming that the
corresponding fixed-size structures are present. Truncated records can
therefore cause out-of-bounds field reads while probing or setting up
virtualization data.

Check each typed MADT record before its first field access. Also reject
a short distributor record in the version validator, which runs before
the probe callback.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 6e1fa5b247fc..0588d6d83b31 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -2299,6 +2299,9 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header,
 			(struct acpi_madt_generic_redistributor *)header;
 	void __iomem *redist_base;
 
+	if (BAD_MADT_ENTRY(redist, end))
+		return -EINVAL;
+
 	redist_base = ioremap(redist->base_address, redist->length);
 	if (!redist_base) {
 		pr_err("Couldn't map GICR region @%llx\n", redist->base_address);
@@ -2325,6 +2328,9 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
 	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
 	void __iomem *redist_base;
 
+	if (BAD_MADT_ENTRY(gicc, end))
+		return -EINVAL;
+
 	/* Neither enabled or online capable means it doesn't exist, skip it */
 	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
 		return 0;
@@ -2391,6 +2397,9 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,
 	struct acpi_madt_generic_interrupt *gicc =
 				(struct acpi_madt_generic_interrupt *)header;
 
+	if (BAD_MADT_ENTRY(gicc, end))
+		return -EINVAL;
+
 	/*
 	 * If GICC is enabled and has valid gicr base address, then it means
 	 * GICR base is presented via GICC. The redistributor is only known to
@@ -2437,6 +2446,9 @@ static bool __init acpi_validate_gic_table(struct acpi_subtable_header *header,
 	int count;
 
 	dist = (struct acpi_madt_generic_distributor *)header;
+	if (header->length < sizeof(*dist))
+		return false;
+
 	if (dist->version != ape->driver_data)
 		return false;
 
@@ -2457,6 +2469,9 @@ static int __init gic_acpi_parse_virt_madt_gicc(union acpi_subtable_headers *hea
 	int maint_irq_mode;
 	static int first_madt = true;
 
+	if (BAD_MADT_ENTRY(gicc, end))
+		return -EINVAL;
+
 	if (!(gicc->flags &
 	      (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
 		return 0;
@@ -2547,6 +2562,9 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
 
 	/* Get distributor base address */
 	dist = (struct acpi_madt_generic_distributor *)header;
+	if (BAD_MADT_ENTRY(dist, end))
+		return -EINVAL;
+
 	acpi_data.dist_base = ioremap(dist->base_address,
 				      ACPI_GICV3_DIST_MEM_SIZE);
 	if (!acpi_data.dist_base) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/6] irqchip/gic-v5-its: validate MADT translate frames
  2026-10-16 14:13 [PATCH 0/6] irqchip: validate ACPI MADT entries before use Jiale Yao
  2026-10-16 14:13 ` [PATCH 2/6] irqchip/gic-v3-its: validate MADT ITS entries Jiale Yao
  2026-10-16 14:13 ` [PATCH 3/6] irqchip/gic-v3: validate MADT records before use Jiale Yao
@ 2026-10-16 14:13 ` Jiale Yao
  2 siblings, 0 replies; 6+ messages in thread
From: Jiale Yao @ 2026-10-16 14:13 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Marc Zyngier, Thomas Gleixner, Radu Rendec,
	linux-arm-kernel, linux-kernel
  Cc: Jiale Yao

The GICv5 ITS translate-frame callback accesses IDs and the base address
without checking that the MADT entry contains the complete fixed-size
record. A truncated firmware entry can therefore cause out-of-bounds
reads.

Reject incomplete translate-frame entries before using their fields.

Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/irqchip/irq-gic-v5-its.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
index 28e39b065de0..dad3b95ba150 100644
--- a/drivers/irqchip/irq-gic-v5-its.c
+++ b/drivers/irqchip/irq-gic-v5-its.c
@@ -1233,6 +1233,9 @@ static int __init gic_acpi_parse_madt_its_translate(union acpi_subtable_headers
 	int err;
 
 	its_frame = (struct acpi_madt_gicv5_translate_frame *)header;
+	if (BAD_MADT_ENTRY(its_frame, end))
+		return -EINVAL;
+
 	if (its_frame->linked_translator_id != current_its_entry->translator_id)
 		return 0;
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/6] irqchip/gic-v3: validate MADT records before use
  2026-10-16 14:13 ` [PATCH 3/6] irqchip/gic-v3: validate MADT records before use Jiale Yao
@ 2026-10-16 14:36   ` Marc Zyngier
  2026-10-16 14:52     ` jiale yao
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2026-10-16 14:36 UTC (permalink / raw)
  To: Jiale Yao; +Cc: Thomas Gleixner, Radu Rendec, linux-arm-kernel, linux-kernel

On Fri, 16 Oct 2026 15:13:54 +0100,
Jiale Yao <yaojiale02@163.com> wrote:
> 
> The GICv3 ACPI paths cast generic MADT headers to distributor,
> redistributor, and CPU interface records before confirming that the
> corresponding fixed-size structures are present. Truncated records can
> therefore cause out-of-bounds field reads while probing or setting up
> virtualization data.
>
> Check each typed MADT record before its first field access. Also reject
> a short distributor record in the version validator, which runs before
> the probe callback.
> 
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6e1fa5b247fc..0588d6d83b31 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2299,6 +2299,9 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header,
>  			(struct acpi_madt_generic_redistributor *)header;
>  	void __iomem *redist_base;
>  
> +	if (BAD_MADT_ENTRY(redist, end))
> +		return -EINVAL;
> +
>  	redist_base = ioremap(redist->base_address, redist->length);
>  	if (!redist_base) {
>  		pr_err("Couldn't map GICR region @%llx\n", redist->base_address);
> @@ -2325,6 +2328,9 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
>  	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
>  	void __iomem *redist_base;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +
>  	/* Neither enabled or online capable means it doesn't exist, skip it */
>  	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>  		return 0;
> @@ -2391,6 +2397,9 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,
>  	struct acpi_madt_generic_interrupt *gicc =
>  				(struct acpi_madt_generic_interrupt *)header;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +
>  	/*
>  	 * If GICC is enabled and has valid gicr base address, then it means
>  	 * GICR base is presented via GICC. The redistributor is only known to
> @@ -2437,6 +2446,9 @@ static bool __init acpi_validate_gic_table(struct acpi_subtable_header *header,
>  	int count;
>  
>  	dist = (struct acpi_madt_generic_distributor *)header;
> +	if (header->length < sizeof(*dist))
> +		return false;
> +

Why is this open-coded instead of using the checking macro?

>  	if (dist->version != ape->driver_data)
>  		return false;
>  
> @@ -2457,6 +2469,9 @@ static int __init gic_acpi_parse_virt_madt_gicc(union acpi_subtable_headers *hea
>  	int maint_irq_mode;
>  	static int first_madt = true;
>  
> +	if (BAD_MADT_ENTRY(gicc, end))
> +		return -EINVAL;
> +

Why isn't this using BAD_MADT_GICC_ENTRY()?

>  	if (!(gicc->flags &
>  	      (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>  		return 0;
> @@ -2547,6 +2562,9 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
>  
>  	/* Get distributor base address */
>  	dist = (struct acpi_madt_generic_distributor *)header;
> +	if (BAD_MADT_ENTRY(dist, end))
> +		return -EINVAL;
> +

Why does this need to be re-checked if the validation phased has succeeded?

>  	acpi_data.dist_base = ioremap(dist->base_address,
>  				      ACPI_GICV3_DIST_MEM_SIZE);
>  	if (!acpi_data.dist_base) {

Thanks,

	M.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re:Re: [PATCH 3/6] irqchip/gic-v3: validate MADT records before use
  2026-10-16 14:36   ` Marc Zyngier
@ 2026-10-16 14:52     ` jiale yao
  0 siblings, 0 replies; 6+ messages in thread
From: jiale yao @ 2026-10-16 14:52 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Thomas Gleixner, Radu Rendec, linux-arm-kernel, linux-kernel

Hi Marc,
At 2026-10-16 22:36:50, "Marc Zyngier" <maz@kernel.org> wrote:
>On Fri, 16 Oct 2026 15:13:54 +0100,
>Jiale Yao <yaojiale02@163.com> wrote:
>> 
>> The GICv3 ACPI paths cast generic MADT headers to distributor,
>> redistributor, and CPU interface records before confirming that the
>> corresponding fixed-size structures are present. Truncated records can
>> therefore cause out-of-bounds field reads while probing or setting up
>> virtualization data.
>>
>> Check each typed MADT record before its first field access. Also reject
>> a short distributor record in the version validator, which runs before
>> the probe callback.
>> 
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
>> index 6e1fa5b247fc..0588d6d83b31 100644
>> --- a/drivers/irqchip/irq-gic-v3.c
>> +++ b/drivers/irqchip/irq-gic-v3.c
>> @@ -2299,6 +2299,9 @@ gic_acpi_parse_madt_redist(union acpi_subtable_headers *header,
>>  			(struct acpi_madt_generic_redistributor *)header;
>>  	void __iomem *redist_base;
>>  
>> +	if (BAD_MADT_ENTRY(redist, end))
>> +		return -EINVAL;
>> +
>>  	redist_base = ioremap(redist->base_address, redist->length);
>>  	if (!redist_base) {
>>  		pr_err("Couldn't map GICR region @%llx\n", redist->base_address);
>> @@ -2325,6 +2328,9 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,
>>  	u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
>>  	void __iomem *redist_base;
>>  
>> +	if (BAD_MADT_ENTRY(gicc, end))
>> +		return -EINVAL;
>> +
>>  	/* Neither enabled or online capable means it doesn't exist, skip it */
>>  	if (!(gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>>  		return 0;
>> @@ -2391,6 +2397,9 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,
>>  	struct acpi_madt_generic_interrupt *gicc =
>>  				(struct acpi_madt_generic_interrupt *)header;
>>  
>> +	if (BAD_MADT_ENTRY(gicc, end))
>> +		return -EINVAL;
>> +
>>  	/*
>>  	 * If GICC is enabled and has valid gicr base address, then it means
>>  	 * GICR base is presented via GICC. The redistributor is only known to
>> @@ -2437,6 +2446,9 @@ static bool __init acpi_validate_gic_table(struct acpi_subtable_header *header,
>>  	int count;
>>  
>>  	dist = (struct acpi_madt_generic_distributor *)header;
>> +	if (header->length < sizeof(*dist))
>> +		return false;
>> +
>
>Why is this open-coded instead of using the checking macro?

Agreed. I will use BAD_MADT_ENTRY() with the end of the current record instead of open-coding the
minimum-size check.

>
>>  	if (dist->version != ape->driver_data)
>>  		return false;
>>  
>> @@ -2457,6 +2469,9 @@ static int __init gic_acpi_parse_virt_madt_gicc(union acpi_subtable_headers *hea
>>  	int maint_irq_mode;
>>  	static int first_madt = true;
>>  
>> +	if (BAD_MADT_ENTRY(gicc, end))
>> +		return -EINVAL;
>> +
>
>Why isn't this using BAD_MADT_GICC_ENTRY()?

Good point. GICC records have a version-dependent size, and requiring sizeof(*gicc) can reject valid records
from older ACPI revisions. I will use BAD_MADT_GICC_ENTRY() here and in the other two GICC callbacks added by
this patch.

>
>>  	if (!(gicc->flags &
>>  	      (ACPI_MADT_ENABLED | ACPI_MADT_GICC_ONLINE_CAPABLE)))
>>  		return 0;
>> @@ -2547,6 +2562,9 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
>>  
>>  	/* Get distributor base address */
>>  	dist = (struct acpi_madt_generic_distributor *)header;
>> +	if (BAD_MADT_ENTRY(dist, end))
>> +		return -EINVAL;
>> +
>
>Why does this need to be re-checked if the validation phased has succeeded?

It does not. The probe callback is invoked with the same record immediately after acpi_validate_gic_table()
succeeds, so the check in gic_acpi_init() is redundant. I will remove it and keep the distributor-size
validation in the validation callback.

>
>>  	acpi_data.dist_base = ioremap(dist->base_address,
>>  				      ACPI_GICV3_DIST_MEM_SIZE);
>>  	if (!acpi_data.dist_base) {
>
>Thanks,
>
>	M.
>
>-- 
>Without deviation from the norm, progress is not possible.

I will fix them in v2, thanks for your review.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-16 14:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 14:13 [PATCH 0/6] irqchip: validate ACPI MADT entries before use Jiale Yao
2026-10-16 14:13 ` [PATCH 2/6] irqchip/gic-v3-its: validate MADT ITS entries Jiale Yao
2026-10-16 14:13 ` [PATCH 3/6] irqchip/gic-v3: validate MADT records before use Jiale Yao
2026-10-16 14:36   ` Marc Zyngier
2026-10-16 14:52     ` jiale yao
2026-10-16 14:13 ` [PATCH 4/6] irqchip/gic-v5-its: validate MADT translate frames Jiale Yao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox