Re: [PATCH v2 1/2] software node: fix refcount leak in software_node_get_next_child()

[Xu Yang]

On Wed, Jun 03, 2026 at 12:40:27PM +0300, Andy Shevchenko wrote:
> On Wed, Jun 03, 2026 at 04:44:31PM +0800, Xu Yang wrote:
> 
> > When a swnode acts as a secondary fwnode and is participates in child
> > iteration, a refcount leak occurs for the last child of the primary
> > fwnode's children.
> > 
> >                    Parent      Child
> >   (Primary fwnode)   FW:   {FW1, FW2, FW3}
> > (Secondary fwnode)   SW:   {}
> > 
> > In this case, FW3's refcount is decremented twice during iteration:
> > 
> >  fwnode_get_next_child_node(FW, FW3)
> >   1. fwnode_call_ptr_op(FW, get_next_child_node, FW3) returns NULL and
> >      decrements FW3's refcount
> >   2. fwnode_call_ptr_op(SW, get_next_child_node, FW3) returns NULL and
> >      decrements FW3's refcount again
> > 
> > The same double-decrement issue occurs when SW has children.
> > 
> > The kernel dump as below:
> > 
> > [   25.435805] OF: ERROR: of_node_release() detected bad of_node_put() on /soc/usb@4c010010/usb@4c100000
> > [   25.445072] CPU: 0 UID: 0 PID: 617 Comm: sh Not tainted 7.1.0-rc4-next-20260522-00011-g7376b330abca #210 PREEMPT
> > [   25.445080] Hardware name: NXP i.MX95 19X19 board (DT)
> > [   25.445083] Call trace:
> > [   25.445086]  show_stack+0x18/0x30 (C)
> > [   25.445101]  dump_stack_lvl+0x60/0x80
> > [   25.445108]  dump_stack+0x18/0x24
> > [   25.445113]  of_node_release+0x158/0x194
> > [   25.445122]  kobject_put+0xa0/0x120
> > [   25.445129]  of_node_put+0x18/0x28
> > [   25.445134]  of_fwnode_put+0x38/0x58
> > [   25.445141]  software_node_get_next_child+0x54/0x15c
> > [   25.445150]  fwnode_get_next_child_node+0x70/0x94
> > [   25.445156]  fwnode_get_next_available_child_node+0x34/0x88
> > [   25.445162]  device_links_driver_bound+0x2f4/0x334
> > [   25.445168]  driver_bound+0x68/0xb0
> >                 ...
> > [   25.445258] OF: ERROR: next of_node_put() on this node will result in a kobject warning 'refcount_t: underflow; use-after-free.'
> > 
> > Fix this by ensuring software_node_get_next_child() does not decrement
> > the child's refcount when:
> > - The parent has no children, OR
> > - The parent has children but the input child is not a swnode
> > 
> > This prevents the refcount from being incorrectly decremented for
> > fwnodes that don't belong to the software node hierarchy.
> 
> ...
> 
> >  	struct swnode *p = to_swnode(fwnode);
> >  	struct swnode *c = to_swnode(child);
> >  
> > -	if (!p || list_empty(&p->children) ||
> > -	    (c && list_is_last(&c->entry, &p->children))) {
> > -		fwnode_handle_put(child);
> 
> Wouldn't be better to use swnode_get() / swnode_put() instead?
> *Yes, we might need to add some NULL checks there.

It's not newly added by me. The software_node_get_next_child() has been using
fwnode_handle_get() / fwnode_handle_put() before. In my opinion, this should
be fine since they do the same thing here for a swnode.

Thanks,
Xu Yang

> 
> > +	if (!p || list_empty(&p->children))
> >  		return NULL;
> > -	}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>