Re: [PATCH v2 01/20] ACPI: irqchip: validate MADT entry bounds before probing

[Rafael J. Wysocki (Intel)]

On Tue, Nov 3, 2026 at 3:44 PM Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> wrote:
>
> On Thu, Oct 29, 2026 at 11:52:32AM +0800, Jiale Yao wrote:
> > IRQ chip probing invokes validation and probe callbacks after the common
> > table walker has only established that a generic subtable header is
> > readable. A malformed length can leave callback field accesses outside
> > the MADT.
> >
> > Validate the generic header length and ensure the complete declared
> > entry is contained in the table before invoking IRQ chip callbacks.
> >
> > Fixes: e647b532275b ("ACPI: Add early device probing infrastructure")
> > Cc: stable@xxxxxxxxxxxxxxx
>
> I don't think this patch fixes anything (actually you don't know, it
> might break kernels that are currently booting).
>
> Remove the stable tag (on the whole series, though others might have a
> different opinion). If this gets merged, goes to stable and then
> we discover it triggers a regression (on existing platforms whose
> ACPI tables are broken but the system is working) it would be very
> annoying.
>
> > Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> > ---
> >  drivers/acpi/scan.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 0b17a7e51956..10e8d736f44b 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -2896,6 +2896,13 @@ static DEFINE_MUTEX(acpi_probe_mutex);
> >  static int __init acpi_match_madt(union acpi_subtable_headers *header,
> >                                 const unsigned long end)
> >  {
> > +     unsigned long start = (unsigned long)header;
> > +
> > +     if (start >= end || end - start < sizeof(header->common) ||
> > +         header->common.length < sizeof(header->common) ||
> > +         header->common.length > end - start)
> > +             return -EINVAL;
>
> This is a check that AFAICS could be done in acpi_parse_entries_array() once
> for all (where you also know header->common is what you need checking, btw).
>
> Actually, what's missing there to avoid peppering the kernel with
> BAD_MADT_ENTRY checks is the entry size (that we could add with a LUT).
>
> The downsize is that by centralizing it, if anything breaks (which is
> possible) we have to revert it and therefore remove the check for all
> consumers.
>
> I don't know. Rafael, all, thoughts on this ?

I have concerns similar to yours: It may break systems that currently boot.

That said, I can imagine a hostile host trying to crash a CoCo guest
by exploiting this missing check.

It might be better to try to use the actual length instead of
header->common.length if the latter is too large though.