Re: [PATCH v4 1/3] device property: fix infinite loop in fwnode_for_each_child_node()

[Xu Yang]

On Thu, Jun 11, 2026 at 10:31:06PM +0200, Andy Shevchenko wrote:
> From: Xu Yang <xu.yang_2@xxxxxxx>
> 
> 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()").
> 
> Fixes: 2692c614f8f0 ("device property: Allow secondary lookup in fwnode_get_next_child_node()")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> Tested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Tested-by: Xu Yang <xu.yang_2@xxxxxxx>

Thanks,
Xu Yang

> ---
>  drivers/base/property.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 8e0148a37fff..f7b30d9c8716 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -807,18 +807,31 @@ struct fwnode_handle *
>  fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
>  			   struct fwnode_handle *child)
>  {
> +	const struct fwnode_handle *parent;
> +	struct fwnode_handle *child_parent __free(fwnode_handle) = NULL;
>  	struct fwnode_handle *next;
>  
> -	if (IS_ERR_OR_NULL(fwnode))
> +	/*
> +	 * If this function is in a loop and the previous iteration returned
> +	 * an child from fwnode->secondary, then we need to use the secondary
> +	 * as parent rather than @fwnode.
> +	 */
> +	if (child) {
> +		child_parent = fwnode_get_parent(child);
> +		parent = child_parent;
> +	} else {
> +		parent = fwnode;
> +	}
> +	if (IS_ERR_OR_NULL(parent))
>  		return NULL;
>  
>  	/* Try to find a child in primary fwnode */
> -	next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> +	next = fwnode_call_ptr_op(parent, get_next_child_node, child);
>  	if (next)
>  		return next;
>  
>  	/* When no more children in primary, continue with secondary */
> -	return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
> +	return fwnode_get_next_child_node(parent->secondary, NULL);
>  }
>  EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
>  
> -- 
> 2.50.1
>