Re: [PATCH] PCI/ACPI: validate _HPX type 3 record lengths

[Rafael J. Wysocki (Intel)]

On Tue, Nov 3, 2026 at 12:41 PM Kang Chen <k.chen@xxxxxxxxxxxxxxxx> wrote:
>
> program_type3_hpx_record() computes the expected record length as
> 3 + desc_count * 14 in 32 bits.  A firmware-provided desc_count of
> 0x80000000 makes this wrap to 3, so the package count check passes for
> a three-element record and the loop then walks far beyond the elements
> array, feeding out-of-bounds data into PCI config space writes.  The
> record is also indexed at fields[1] and fields[2] before its length has
> been checked.
>
> Reject records with fewer than three elements, verify that fields[1]
> and fields[2] are integers, and reject desc_count values for which the
> expected length would overflow.  Reject revision or descriptor count
> values that do not fit in 32 bits instead of silently truncating them.
> Use an unsigned loop index and advance the register pointer explicitly
> so the walk cannot overflow either.  In acpi_run_hpx(), reject records
> that do not have at least the two elements needed to read the type and
> revision.
>
> Fixes: f873c51a155a ("PCI/ACPI: Implement _HPX Type 3 Setting Record")
> Signed-off-by: Kang Chen <k.chen@xxxxxxxxxxxxxxxx>

>From the ACPI support perspective

Acked-by: Rafael J. Wysocki (Intel) <rafael@xxxxxxxxxx>

> ---
>  drivers/pci/pci-acpi.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 1150f2fbabf4..b15700523810 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -607,12 +607,28 @@ static acpi_status program_type3_hpx_record(struct pci_dev *dev,
>         u32 desc_count, expected_length, revision;
>         union acpi_object *reg_fields;
>         struct hpx_type3 hpx3;
> -       int i;
> +       u32 i;
> +
> +       if (record->package.count < 3)
> +               return AE_ERROR;
> +
> +       if (fields[1].type != ACPI_TYPE_INTEGER ||
> +           fields[2].type != ACPI_TYPE_INTEGER)
> +               return AE_ERROR;
> +
> +       if (fields[1].integer.value > U32_MAX)
> +               return AE_ERROR;
>
>         revision = fields[1].integer.value;
>         switch (revision) {
>         case 1:
> +               if (fields[2].integer.value > U32_MAX)
> +                       return AE_ERROR;
> +
>                 desc_count = fields[2].integer.value;
> +               if (desc_count > (UINT_MAX - 3) / 14)
> +                       return AE_ERROR;
> +
>                 expected_length = 3 + desc_count * 14;
>
>                 if (record->package.count != expected_length)
> @@ -622,10 +638,11 @@ static acpi_status program_type3_hpx_record(struct pci_dev *dev,
>                         if (fields[i].type != ACPI_TYPE_INTEGER)
>                                 return AE_ERROR;
>
> +               reg_fields = fields + 3;
>                 for (i = 0; i < desc_count; i++) {
> -                       reg_fields = fields + 3 + i * 14;
>                         parse_hpx3_register(&hpx3, reg_fields);
>                         program_hpx_type3(dev, &hpx3);
> +                       reg_fields += 14;
>                 }
>
>                 break;
> @@ -661,7 +678,8 @@ static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle)
>
>         for (i = 0; i < package->package.count; i++) {
>                 record = &package->package.elements[i];
> -               if (record->type != ACPI_TYPE_PACKAGE) {
> +               if (record->type != ACPI_TYPE_PACKAGE ||
> +                   record->package.count < 2) {
>                         status = AE_ERROR;
>                         goto exit;
>                 }
> --
> 2.34.1
>