Re: [PATCH v2 2/2] device property: fix infinite loop in fwnode_for_each_child_node()

[Bartosz Golaszewski]

On Thu, 4 Jun 2026 13:05:23 +0200, Xu Yang <xu.yang_2@xxxxxxxxxxx> said:
> On Wed, Jun 03, 2026 at 12:51:50PM +0300, Andy Shevchenko wrote:
>> On Wed, Jun 03, 2026 at 04:44:32PM +0800, Xu Yang wrote:
>>
>> > When iterate over children of a fwnode that has a secondary fwnode,
>> > fwnode_get_next_child_node() can enter an infinite loop if the secondary
>> > fwnode has more than one child.
>> >
>> >                        Parent        Child
>> >       (Primary fwnode)   FWa:   {FWa1, FWa2, FWa3}
>> >     (Secondary fwnode)   FWb:   {FWb1, FWb2}
>> >
>> > In this case:
>> >
>> >  ┌─> fwnode_get_next_child_node(FWa, FWa1)
>> >  │    - fwnode_call_ptr_op(FWa, get_next_child_node, FWa1) returns FWa2
>> >  │
>> >  │   ...
>> >  │
>> >  │   fwnode_get_next_child_node(FWa, FWa3)
>> >  │    - fwnode_call_ptr_op(FWa, get_next_child_node, FWa3) returns NULL
>> >  │    - fwnode_call_ptr_op(FWb, get_next_child_node, FWa3) returns FWb1
>> >  │
>> >  │   fwnode_get_next_child_node(FWa, FWb1)
>> >  │    - fwnode_call_ptr_op(FWa, get_next_child_node, FWb1) returns FWa1
>> >  └────┘
>> >
>> > This cause fwnode_for_each_child_node() to loop indefinitely, reapeatedly
>> > output {FWa1, FWa2, FWa3, FWb1, FWa1, ...}.
>> >
>> > The root cause is that when the current child (FWb1) belongs to the
>> > secondary fwnode, calling get_next_child_node() on the parimary fwnode
>> > incorrectly returns the first child (FWa1) again instead of NULL.
>> >
>> > Fix this by dynamically checking the parent fwnode of the current child
>> > before calling get_next_child_node(). This approach follows the pattern
>> > established in commit b5b41ab6b0c1 ("device property: Check
>> > fwnode->secondary in fwnode_graph_get_next_endpoint()").
>>
>> ...
>>
>> TBH, this code becomes twisted and complicated. Can we add some test cases to
>> show the problem? Also we need to add other possible combinations (somewhat
>> about ~5-6) of the different types of fwnode in a relationship.
>
> I agree that adding test cases would be helpful. But It's not straightforward to
> get swnode refcount as swnode is an internal structure. Any suggestions on this?
>

You should be able to replicate the problem with the firmware node API without
accessing the internal swnode structure. You can use dummy OF nodes as the
primary fwnodes.

Bart