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

[Xu Yang]

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?

Thanks,
Xu Yang

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>