On 2025/5/8 10:30, Jeremy Linton wrote:
> The original PPTT code had a bug where the processor subtable length
> was not correctly validated when encountering a truncated
> acpi_pptt_processor node.
>
> Commit 7ab4f0e37a0f4 ("ACPI PPTT: Fix coding mistakes in a couple of
> sizeof() calls") attempted to fix this by validating the size is as
> large as the acpi_pptt_processor node structure. This introduced a
> regression where the last processor node in the PPTT table is ignored
> if it doesn't contain any private resources. That results errors like:
>
> ACPI PPTT: PPTT table found, but unable to locate core XX (XX)
> ACPI: SPE must be homogeneous
>
> Furthermore, it fail in a common case where the node length isn't
> equal to the acpi_pptt_processor structure size, leaving the original
> bug in a modified form.
>
> Correct the regression by adjusting the loop termination conditions as
> suggested by the bug reporters. An additional check performed after
> the subtable node type is detected, validates the acpi_pptt_processor
> node is fully contained in the PPTT table. Repeating the check in
> acpi_pptt_leaf_node() is largely redundant as the node is already
> known to be fully contained in the table.
>
> The case where a final truncated node's parent property is accepted,
> but the node itself is rejected should not be considered a bug.
>
> Fixes: 7ab4f0e37a0f4 ("ACPI PPTT: Fix coding mistakes in a couple of sizeof() calls")
> Reported-by: Maximilian Heyne <mheyne@xxxxxxxxx>
> Closes: https://lore.kernel.org/linux-acpi/20250506-draco-taped-15f475cd@mheyne-amazon/
> Reported-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
Thanks for the fix. The last CPU in the PPTT can be parsed on my board with this.
Tested-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-acpi/20250507035124.28071-1-yangyicong@xxxxxxxxxx/
> Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx>
> Cc: Jean-Marc Eurin <jmeurin@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> ---
> drivers/acpi/pptt.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> index f73ce6e13065..54676e3d82dd 100644
> --- a/drivers/acpi/pptt.c
> +++ b/drivers/acpi/pptt.c
> @@ -231,16 +231,18 @@ static int acpi_pptt_leaf_node(struct acpi_table_header *table_hdr,
> sizeof(struct acpi_table_pptt));
> proc_sz = sizeof(struct acpi_pptt_processor);
>
> - while ((unsigned long)entry + proc_sz < table_end) {
> + /* ignore subtable types that are smaller than a processor node */
> + while ((unsigned long)entry + proc_sz <= table_end) {
> cpu_node = (struct acpi_pptt_processor *)entry;
> +
> if (entry->type == ACPI_PPTT_TYPE_PROCESSOR &&
> cpu_node->parent == node_entry)
> return 0;
> if (entry->length == 0)
> return 0;
> +
> entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
> entry->length);
> -
> }
> return 1;
> }
> @@ -273,15 +275,18 @@ static struct acpi_pptt_processor *acpi_find_processor_node(struct acpi_table_he
> proc_sz = sizeof(struct acpi_pptt_processor);
>
> /* find the processor structure associated with this cpuid */
> - while ((unsigned long)entry + proc_sz < table_end) {
> + while ((unsigned long)entry + proc_sz <= table_end) {
> cpu_node = (struct acpi_pptt_processor *)entry;
>
> if (entry->length == 0) {
> pr_warn("Invalid zero length subtable\n");
> break;
> }
> + /* entry->length may not equal proc_sz, revalidate the processor structure length */
> if (entry->type == ACPI_PPTT_TYPE_PROCESSOR &&
> acpi_cpu_id == cpu_node->acpi_processor_id &&
> + (unsigned long)entry + entry->length <= table_end &&
> + entry->length == proc_sz + cpu_node->number_of_priv_resources * sizeof(u32) &&
> acpi_pptt_leaf_node(table_hdr, cpu_node)) {
> return (struct acpi_pptt_processor *)entry;
> }
>
