On Mon, Sep 25, 2023 at 10:03 PM Zhang, Rui <rui.zhang@xxxxxxxxx> wrote:
>
> 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?
I'm not aware of a bug filed a kernel bugzilla, however I found
mentions of the issue online:
https://forum.proxmox.com/threads/acpi-fpdt-duplicate-resume-performance-record-found.114530/
> > ---
> > 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.
Here's FPDT dump:
00000000: 4650 4454 4400 0000 018c 414c 4153 4b41 FPDTD.....ALASKA
00000010: 4120 4d20 4920 0000 0920 0701 414d 4920 A M I ... ..AMI
00000020: 1300 0100 0100 1001 0000 0000 30fe 207f ............0. .
00000030: 0000 0000 0000 1001 0000 0000 54fe 207f ............T. .
00000040: 0000 0000 ....
S3PT at 0x7f20fe30:
7F20FE30: 53 33 50 54 24 00 00 00-00 00 00 00 00 00 18 01 *S3PT$...........*
7F20FE40: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 *................*
7F20FE50: 00 00 00 00
FBPT at 0x7f20fe54:
7F20FE50: xx xx xx xx 46 42 50 54-3C 00 00 00 46 42 50 54 *....FBPT<...FBPT*
7F20FE60: 02 00 30 02 00 00 00 00-00 00 00 00 00 00 00 00 *..0.............*
7F20FE70: 2A A6 BC 6E 0B 00 00 00-1A 44 41 70 0B 00 00 00 **..n.....DAp....*
7F20FE80: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 *................*
It looks like subtables are not usable. S3PT subtable has the first
record with zero len, and FBPT has its signature again instead of the
first record header.
So yeah, I agree that FPDT is not usabled in this case, and it
shouldn't be processed further.
> 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?
Agree, I'll implement it in v2.
Regards,
Vasily
