On Mon, Sep 25, 2023 at 10:03 PM Zhang, Rui rui.zhang@intel.com 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@vger.kernel.org
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
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-rec...
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 recordfound.\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
- return an error here rather than break and return 0.
- add the error handling for fpdt_process_subtable() failures.
what do you think?
Agree, I'll implement it in v2.
Regards, Vasily