[PATCH] powerpc/pseries: fix of_read_drc_info_cell() to point at next record

[Michael Ellerman]

Tyrel Datwyler <tyreld at linux.ibm.com> writes:
> On 3/10/20 10:25 AM, Nathan Lynch wrote:
>> Tyrel Datwyler <tyreld at linux.ibm.com> writes:
>>> The expectation is that when calling of_read_drc_info_cell()
>>> repeatedly to parse multiple drc-info records that the in/out curval
>>> parameter points at the start of the next record on return. However,
>>> the current behavior has curval still pointing at the final value of
>>> the record just parsed. The result of which is that if the
>>> ibm,drc-info property contains multiple properties the parsed value
>>> of the drc_type for any record after the first has the power_domain
>>> value of the previous record appended to the type string.
>>>
>>> Ex: observed the following 0xffffffff prepended to PHB
>>>
>>> [   69.485037] drc-info: type: \xff\xff\xff\xffPHB, prefix: PHB , index_start: 0x20000001
>>> [   69.485038] drc-info: suffix_start: 1, sequential_elems: 3072, sequential_inc: 1
>>> [   69.485038] drc-info: power-domain: 0xffffffff, last_index: 0x20000c00
>>>
>>> Fix by incrementing curval past the power_domain value to point at
>>> drc_type string of next record.
>>>
>>> Fixes: a29396653b8bf ("pseries/drc-info: Search DRC properties for CPU indexes")
>> 
>> I have a different commit hash for that:
>> e83636ac3334 pseries/drc-info: Search DRC properties for CPU indexes
>
> Oof, looks like I grabbed the commit hash from the SLES 15 SP1 branch. You have
> the correct upstream commit.
>
> Fixes: e83636ac3334 ("pseries/drc-info: Search DRC properties for CPU indexes")
>
> Michael, let me know if you want me to resubmit, or if you will fixup the Fixes
> tag on your end?

I can update the Fixes tag.

What's the practical effect of this bug? It seems like it should break
all the hotplug stuff, but presumably it doesn't in practice for some
reason?

It would also be *really* nice if we had some unit tests for this
parsing code, it's demonstrably very bug prone.

cheers