On 26/05/17 17:40, Lorenzo Pieralisi wrote: > The BAD_MADT_GICC_ENTRY() macro checks if a GICC MADT entry passes > muster from an ACPI specification standpoint. Current macro detects the > MADT GICC entry length through ACPI firmware version (it changed from 76 > to 80 bytes in the transition from ACPI 5.1 to ACPI 6.0 specification) > but always uses (erroneously) the ACPICA (latest) struct (ie struct > acpi_madt_generic_interrupt - that is 80-bytes long) length to check if > the current GICC entry memory record exceeds the MADT table end in > memory as defined by the MADT table header itself, which may result in > false negatives depending on the ACPI firmware version and how the MADT > entries are laid out in memory (ie on ACPI 5.1 firmware MADT GICC > entries are 76 bytes long, so by adding 80 to a GICC entry start address > in memory the resulting address may well be past the actual MADT end, > triggering a false negative). > > Fix the BAD_MADT_GICC_ENTRY() macro by reshuffling the condition checks > and update them to always use the firmware version specific MADT GICC > entry length in order to carry out boundary checks. > > Fixes: b6cfb277378e ("ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro") > Reported-by: Julien Grall <julien.grall@xxxxxxx> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > Cc: Will Deacon <will.deacon@xxxxxxx> > Cc: Julien Grall <julien.grall@xxxxxxx> > Cc: Hanjun Guo <hanjun.guo@xxxxxxxxxx> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > Cc: Al Stone <ahs3@xxxxxxxxxx> > Cc: Marc Zyngier <marc.zyngier@xxxxxxx> > --- > arch/arm64/include/asm/acpi.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h > index 0e99978..59cca1d 100644 > --- a/arch/arm64/include/asm/acpi.h > +++ b/arch/arm64/include/asm/acpi.h > @@ -23,9 +23,9 @@ > #define ACPI_MADT_GICC_LENGTH \ > (acpi_gbl_FADT.header.revision < 6 ? 76 : 80) > > -#define BAD_MADT_GICC_ENTRY(entry, end) \ > - (!(entry) || (unsigned long)(entry) + sizeof(*(entry)) > (end) || \ > - (entry)->header.length != ACPI_MADT_GICC_LENGTH) > +#define BAD_MADT_GICC_ENTRY(entry, end) \ > + (!(entry) || (entry)->header.length != ACPI_MADT_GICC_LENGTH || \ > + (unsigned long)(entry) + ACPI_MADT_GICC_LENGTH > (end)) > > /* Basic configuration for ACPI */ > #ifdef CONFIG_ACPI > Acked-by: Marc Zyngier <marc.zyngier@xxxxxxx> M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html