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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Sep 2023 05:02:47 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "rafael@...nel.org" <rafael@...nel.org>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "anarsoul@...il.com" <anarsoul@...il.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
CC:     "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] ACPI: FPDT: break out of the loop if record length is
 zero

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@...r.kernel.org
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@...il.com>

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) {