Re: [PATCH] ACPI: FPDT: validate table and record lengths

On Mon, Jul 6, 2026 at 11:41 AM Pengpeng Hou <[email protected]> wrote:
>
> FPDT records are supplied by firmware and are walked through
> length fields stored in the table itself.
>
> Check the main-table entry length before reading an entry, check
> the mapped subtable length before remapping it, and check each
> record length before saving a typed record pointer for sysfs.
>
> This keeps malformed firmware records from being interpreted as
> complete resume, suspend, or boot records when the current item
> is shorter than the structure consumed by the driver.
>
> Signed-off-by: Pengpeng Hou <[email protected]>
> ---
>  drivers/acpi/acpi_fpdt.c | 53 +++++++++++++++++++++++++++++++++++-----
>  1 file changed, 47 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> index e75dd28d31a9..79191ed20f76 100644
> --- a/drivers/acpi/acpi_fpdt.c
> +++ b/drivers/acpi/acpi_fpdt.c
> @@ -168,7 +168,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
>         struct fpdt_subtable_header *subtable_header;
>         struct fpdt_record_header *record_header;
>         char *signature = (subtable_type == SUBTABLE_FBPT ? "FBPT" : "S3PT");
> -       u32 length, offset;
> +       u32 length, offset, remaining;
>         int result;
>
>         if (!fpdt_address_valid(address)) {
> @@ -182,10 +182,17 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
>
>         if (strncmp((char *)&subtable_header->signature, signature, 4)) {
>                 pr_info(FW_BUG "subtable signature and type mismatch!\n");
> +               acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
>                 return -EINVAL;
>         }
>
>         length = subtable_header->length;
> +       if (length < sizeof(*subtable_header)) {
> +               pr_err(FW_BUG "Invalid FPDT subtable length %u.\n", length);
> +               acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
> +               return -EINVAL;
> +       }
> +
>         acpi_os_unmap_memory(subtable_header, sizeof(*subtable_header));
>
>         subtable_header = acpi_os_map_memory(address, length);
> @@ -194,17 +201,29 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
>
>         offset = sizeof(*subtable_header);
>         while (offset < length) {
> -               record_header = (void *)subtable_header + offset;
> -               offset += record_header->length;
> +               remaining = length - offset;
> +               if (remaining < sizeof(*record_header)) {
> +                       pr_err(FW_BUG "Truncated FPDT record header.\n");
> +                       result = -EINVAL;
> +                       goto err;
> +               }
>
> -               if (!record_header->length) {
> -                       pr_err(FW_BUG "Zero-length record found in FPTD.\n");
> +               record_header = (void *)subtable_header + offset;
> +               if (record_header->length < sizeof(*record_header) ||
> +                   record_header->length > remaining) {
> +                       pr_err(FW_BUG "Invalid FPDT record length %u.\n",
> +                              record_header->length);
>                         result = -EINVAL;
>                         goto err;
>                 }
> +               offset += record_header->length;
>
>                 switch (record_header->type) {
>                 case RECORD_S3_RESUME:
> +                       if (record_header->length < sizeof(*record_resume)) {
> +                               result = -EINVAL;
> +                               goto err;
> +                       }
>                         if (subtable_type != SUBTABLE_S3PT) {
>                                 pr_err(FW_BUG "Invalid record %d for subtable %s\n",
>                                      record_header->type, signature);
> @@ -221,6 +240,10 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
>                                 goto err;
>                         break;
>                 case RECORD_S3_SUSPEND:
> +                       if (record_header->length < sizeof(*record_suspend)) {
> +                               result = -EINVAL;
> +                               goto err;
> +                       }
>                         if (subtable_type != SUBTABLE_S3PT) {
>                                 pr_err(FW_BUG "Invalid %d for subtable %s\n",
>                                      record_header->type, signature);
> @@ -236,6 +259,10 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
>                                 goto err;
>                         break;
>                 case RECORD_BOOT:
> +                       if (record_header->length < sizeof(*record_boot)) {
> +                               result = -EINVAL;
> +                               goto err;
> +                       }
>                         if (subtable_type != SUBTABLE_FBPT) {
>                                 pr_err(FW_BUG "Invalid %d for subtable %s\n",
>                                      record_header->type, signature);
> @@ -317,7 +344,21 @@ static int __init acpi_init_fpdt(void)
>         }
>
>         while (offset < header->length) {
> +               if (header->length - offset < sizeof(*subtable)) {
> +                       pr_err(FW_BUG "Truncated FPDT subtable entry.\n");
> +                       result = -EINVAL;
> +                       goto err_subtable;
> +               }
> +
>                 subtable = (void *)header + offset;
> +               if (subtable->length < sizeof(*subtable) ||
> +                   subtable->length > header->length - offset) {
> +                       pr_err(FW_BUG "Invalid FPDT subtable entry length %u.\n",
> +                              subtable->length);
> +                       result = -EINVAL;
> +                       goto err_subtable;
> +               }
> +
>                 switch (subtable->type) {
>                 case SUBTABLE_FBPT:
>                 case SUBTABLE_S3PT:
> @@ -330,7 +371,7 @@ static int __init acpi_init_fpdt(void)
>                         /* Other types are reserved in ACPI 6.4 spec. */
>                         break;
>                 }
> -               offset += sizeof(*subtable);
> +               offset += subtable->length;
>         }
>         return 0;
>  err_subtable:
> --

Applied as 7.3 material, thanks!