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

[Xu Yang]

On Thu, Jun 04, 2026 at 06:43:38AM -0700, Bartosz Golaszewski wrote:
> 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.

Got it. I thought it's the refcount leak one. Then no needs to get refcount.

Thanks,
Xu Yang

> 
> Bart