On Wed, May 07, 2025 at 10:25:25AM -0500, Jeremy Linton wrote:
Hi,
On 5/6/25 8:13 AM, Heyne, Maximilian wrote:
Commit 7ab4f0e37a0f ("ACPI PPTT: Fix coding mistakes in a couple of sizeof() calls") corrects the processer entry size but unmasked a longer standing bug where the last entry in the structure can get skipped due to an off-by-one mistake if the last entry ends exactly at the end of the ACPI subtable.
The error manifests for instance on EC2 Graviton Metal instances with
ACPI PPTT: PPTT table found, but unable to locate core 63 (63) [...] ACPI: SPE must be homogeneous
Fixes: 2bd00bcd73e5 ("ACPI/PPTT: Add Processor Properties Topology Table parsing") Cc: stable@vger.kernel.org Signed-off-by: Maximilian Heyne mheyne@amazon.de
drivers/acpi/pptt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c index f73ce6e13065d..4364da90902e5 100644 --- a/drivers/acpi/pptt.c +++ b/drivers/acpi/pptt.c @@ -231,7 +231,7 @@ static int acpi_pptt_leaf_node(struct acpi_table_header *table_hdr, sizeof(struct acpi_table_pptt)); proc_sz = sizeof(struct acpi_pptt_processor);
This isn't really right, it should be struct acpi_subtable_header, then once the header is safe, pull the length from it.
Ah OK. Sorry I wasn't able to understand your point earlier. I get it now.
But just for sake of argument here, accessing entry->length before doing some sanity check is also risky. So ideally we should be checking if entry + entry->length <= table_end right ?
But then, really if we are trying to fix the original bug that the table could be shorter than the data in it suggests, the struct acpi_pptt_processor length plus its resources needs to be checked once the subtype is known to be a processor node.
Indeed.
Otherwise the original sizeof * change isn't really fixing anything.
How about extending the check for entry->length ? Do you think it will be any better ? The entry pointer is anyway updated to jump entry->length ahead at the end of the loop.
Regards, Sudeep
-->8
@@ -276,7 +276,7 @@ static struct acpi_pptt_processor *acpi_find_processor_node(struct acpi_table_he while ((unsigned long)entry + proc_sz <= table_end) { cpu_node = (struct acpi_pptt_processor *)entry;
- if (entry->length == 0) { + if (!entry->length || entry->length < proc_sz) { pr_warn("Invalid zero length subtable\n"); break; }