Skip to content

Commit

Permalink
ACPI: FPDT: properly handle invalid FPDT subtables
Browse files Browse the repository at this point in the history
commit a83c68a upstream.

Buggy BIOSes may have invalid FPDT subtables, e.g. on my hardware:

S3PT subtable:

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

Here the first record has zero length.

FBPT subtable:

7F20FE50:             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  *................*

And here FBPT table has FBPT signature repeated instead of the first
record.

Current code will be looping indefinitely due to zero length records, so
break out of the loop if record length is zero.

While we are here, add proper handling for fpdt_process_subtable()
failures.

Fixes: d1eb86e ("ACPI: tables: introduce support for FPDT table")
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
[ rjw: Comment edit, added empty code lines ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
anarsoul authored and gregkh committed Nov 28, 2023
1 parent 95e747c commit c754a6f
Showing 1 changed file with 37 additions and 8 deletions.
45 changes: 37 additions & 8 deletions drivers/acpi/acpi_fpdt.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,19 @@ 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_err(FW_BUG "Zero-length record found in FPTD.\n");
result = -EINVAL;
goto err;
}

switch (record_header->type) {
case RECORD_S3_RESUME:
if (subtable_type != SUBTABLE_S3PT) {
pr_err(FW_BUG "Invalid record %d for subtable %s\n",
record_header->type, signature);
return -EINVAL;
result = -EINVAL;
goto err;
}
if (record_resume) {
pr_err("Duplicate resume performance record found.\n");
Expand All @@ -208,7 +215,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
record_resume = (struct resume_performance_record *)record_header;
result = sysfs_create_group(fpdt_kobj, &resume_attr_group);
if (result)
return result;
goto err;
break;
case RECORD_S3_SUSPEND:
if (subtable_type != SUBTABLE_S3PT) {
Expand All @@ -223,13 +230,14 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
record_suspend = (struct suspend_performance_record *)record_header;
result = sysfs_create_group(fpdt_kobj, &suspend_attr_group);
if (result)
return result;
goto err;
break;
case RECORD_BOOT:
if (subtable_type != SUBTABLE_FBPT) {
pr_err(FW_BUG "Invalid %d for subtable %s\n",
record_header->type, signature);
return -EINVAL;
result = -EINVAL;
goto err;
}
if (record_boot) {
pr_err("Duplicate boot performance record found.\n");
Expand All @@ -238,7 +246,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
record_boot = (struct boot_performance_record *)record_header;
result = sysfs_create_group(fpdt_kobj, &boot_attr_group);
if (result)
return result;
goto err;
break;

default:
Expand All @@ -247,6 +255,18 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type)
}
}
return 0;

err:
if (record_boot)
sysfs_remove_group(fpdt_kobj, &boot_attr_group);

if (record_suspend)
sysfs_remove_group(fpdt_kobj, &suspend_attr_group);

if (record_resume)
sysfs_remove_group(fpdt_kobj, &resume_attr_group);

return result;
}

static int __init acpi_init_fpdt(void)
Expand All @@ -255,6 +275,7 @@ static int __init acpi_init_fpdt(void)
struct acpi_table_header *header;
struct fpdt_subtable_entry *subtable;
u32 offset = sizeof(*header);
int result;

status = acpi_get_table(ACPI_SIG_FPDT, 0, &header);

Expand All @@ -263,17 +284,19 @@ static int __init acpi_init_fpdt(void)

fpdt_kobj = kobject_create_and_add("fpdt", acpi_kobj);
if (!fpdt_kobj) {
acpi_put_table(header);
return -ENOMEM;
result = -ENOMEM;
goto err_nomem;
}

while (offset < header->length) {
subtable = (void *)header + offset;
switch (subtable->type) {
case SUBTABLE_FBPT:
case SUBTABLE_S3PT:
fpdt_process_subtable(subtable->address,
result = fpdt_process_subtable(subtable->address,
subtable->type);
if (result)
goto err_subtable;
break;
default:
/* Other types are reserved in ACPI 6.4 spec. */
Expand All @@ -282,6 +305,12 @@ static int __init acpi_init_fpdt(void)
offset += sizeof(*subtable);
}
return 0;
err_subtable:
kobject_put(fpdt_kobj);

err_nomem:
acpi_put_table(header);
return result;
}

fs_initcall(acpi_init_fpdt);

0 comments on commit c754a6f

Please sign in to comment.