Re: [PATCH 1/2] drivers/of: Add of_detach_node_no_notify()

[Haren Myneni <haren@linux.ibm.com>]

On Wed, 2026-09-16 at 04:43 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> - [High] of: state corruption and resource leak via
> of_detach_node_no_notify()
> --
> 
> commit 900ccea33cdd682696cd3ae6fb47477962cee9f0
> Author: Haren Myneni <haren@linux.ibm.com>
> 
>     drivers/of: Add of_detach_node_no_notify()
> 
> This commit introduces the of_detach_node_no_notify() interface. It
> allows
> callers to remove nodes from the device tree without triggering
> reconfiguration
> notifiers, intending to provide a cleanup path when node addition
> fails.
> 
> > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> > index 744ce0e1eb24a..fc6f22371a97c 100644
> > --- a/drivers/of/dynamic.c
> > +++ b/drivers/of/dynamic.c
> [ ... ]
> > @@ -318,6 +318,22 @@ int of_detach_node(struct device_node *np)
> >  }
> >  EXPORT_SYMBOL_GPL(of_detach_node);
> >  
> > +/**
> > + * of_detach_node_no_notify() - "Unplug" a node from the device
> > + * and return without running notifiers.
> > + * @np:	Pointer to the caller's Device Node
> > + */
> > +int of_detach_node_no_notify(struct device_node *np)
> > +{
> > +	mutex_lock(&of_mutex);
> > +	if (!of_node_check_flag(np, OF_DETACHED))
> > +		__of_detach_node(np);
> > +	mutex_unlock(&of_mutex);
> > +
> > +	return 0;
> > +}
> 
> [Severity: High]
> Does this bypass of of_reconfig_notify(OF_RECONFIG_DETACH_NODE) cause
> state
> corruption and resource leaks?
> 
> When of_changeset_apply() fails during the notification phase (for
> example,
> if a subsystem returns an error for OF_RECONFIG_ATTACH_NODE), earlier
> subsystems in the blocking_notifier_call_chain may have already
> successfully
> processed the ATTACH_NODE event and allocated resources such as
> platform
> devices or cached node pointers.
> 
> If of_detach_node_no_notify() is subsequently used to undo the
> changeset, the
> required OF_RECONFIG_DETACH_NODE signal is intentionally suppressed.
> Will this
> leave the successfully attached subsystems blind to the removal,
> resulting in
> permanent resource leaks and potential use-after-free of the device
> nodes via
> dangling pointers?
> 
> This path can be reached through userspace-initiated CPU hotplug
> operations
> (DLPAR) that exercise this error path.

Yes, of_changeset_apply() can add  and have successful notifiers for
some nodes. of_detach_node_no_notify() is called for each node ( its
children first and then the parent node) in dlpar_detach_node() and up
to the caller. 

For CPU nodes, only the parent CPU node has the notifier. and 
of_detach_node_no_notify() is issued only for the failure from the
notifier. So should not be an issue.