Re: [PATCH] ACPI: FPDT: break out of the loop if record length is zero

[Zhang, Rui]

On Mon, 2023-09-25 at 14:40 -0700, Vasily Khoruzhick wrote:
> Buggy BIOSes may have zero-length records in FPDT, table, as a result
s/FPDT, table/FPDT table


> fpdt_process_subtable() spins in eternal loop.
> 
> Break out of the loop if record length is zero.
> 
> 
> Fixes: d1eb86e59be0 ("ACPI: tables: introduce support for FPDT
> table")
> Cc: stable@xxxxxxxxxxxxxxx
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@xxxxxxxxx>

Is there a bugzilla or something where such a buggy BIOS is reported?

> ---
>  drivers/acpi/acpi_fpdt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> index a2056c4c8cb7..53d8f9601a55 100644
> --- a/drivers/acpi/acpi_fpdt.c
> +++ b/drivers/acpi/acpi_fpdt.c
> @@ -194,6 +194,11 @@ static int fpdt_process_subtable(u64 address,
> u32 subtable_type)
>                 record_header = (void *)subtable_header + offset;
>                 offset += record_header->length;
>  
> +               if (!record_header->length) {
> +                       pr_info(FW_BUG "Zero-length record
> found.\n");
> +                       break;

For cases like this, it implies the FPDT table is far from usable and
we should stop processing on such platforms.

So, IMO, it is better to
1. return an error here rather than break and return 0.
2. add the error handling for fpdt_process_subtable() failures.

what do you think?

thanks,
rui

> +               }
> +
>                 switch (record_header->type) {
>                 case RECORD_S3_RESUME:
>                         if (subtable_type != SUBTABLE_S3PT) {