Re: [PATCH] ACPI/PPTT: fix off-by-one error

Sudeep Holla
2025-05-07T16:28:10Z

On Wed, May 07, 2025 at 06:12:40PM +0200, Rafael J. Wysocki wrote:
> On Wed, May 7, 2025 at 5:51 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
> >

[...]
> > So if the bug being fixed is that the length check is validating that
> > the table length is less than the data in the table, that's still a
> > problem because its only validating the processor node without resources.
>
> Admittedly, it is not my code, but I understand this check as a
> termination condition for the loop: If there's not enough space in the
> table to hold a thing that I'm looking for, I may as well bail out.
>
> > AKA the return is still potentially returning a pointer to a structure
> > which may not be entirely contained in the table.
>
> Right, but this check should be made anyway before comparing
> cpu_node->parent to node_entry, when it is known to be a CPU entry
> because otherwise why bother.
>
> Roughly something like this:
>
> proc_sz = sizeof(struct acpi_pptt_processor);
>
> while ((unsigned long)entry + entry->length <= table_end) {

Yes, but in the last/termination run of the loop, entry will be > table_end,
is it safe to access entry->length in that case. That's the point
I was trying to make when I mentioned it is risky to use entry->length
in this check. That location(outside of PPTT) might have a value that may
result in entering the loop. We need to make sure the entry + offset(length)
is within the table_end to access it.

--
Regards,
Sudeep