* [PATCH 1/2] drivers/of: Add of_detach_node_no_notify()
@ 2026-09-16 4:32 Haren Myneni
2026-09-16 4:32 ` [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier Haren Myneni
2026-09-16 4:43 ` [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() sashiko-bot
0 siblings, 2 replies; 8+ messages in thread
From: Haren Myneni @ 2026-09-16 4:32 UTC (permalink / raw)
To: linuxppc-dev
Cc: devicetree, maddy, robh, mpe, npiggin, saravanak, ritesh.list,
hbabu, haren
of_changeset_apply() interface adds nodes for action
OF_RECONFIG_ATTACH_NODE and calls notifiers for each attached node.
Then the caller uses of_detach_node() to release each node and
also calls the corresponding notifier function. In the case of
failure from the add notifier for any node, of_changeset_apply()
returns failure after adding nodes. The current implementation
does not the provide any interface to the caller to remove nodes
from the DT without calling notifiers. It may end up having nodes
in DT even though ADD is not successful.
This patch introduces of_detach_node_no_notify() interface for the
caller to only remove node if it is not attached.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
drivers/of/dynamic.c | 16 ++++++++++++++++
include/linux/of.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 744ce0e1eb24..fc6f22371a97 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;
+}
+EXPORT_SYMBOL_GPL(of_detach_node_no_notify);
+
void __of_prop_free(struct property *prop)
{
kfree(prop->name);
diff --git a/include/linux/of.h b/include/linux/of.h
index b920aac6b975..7a509341e998 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -442,6 +442,7 @@ extern int of_update_property(struct device_node *np, struct property *newprop);
extern int of_attach_node(struct device_node *);
extern int of_detach_node(struct device_node *);
+extern int of_detach_node_no_notify(struct device_node *);
#define of_match_ptr(_ptr) (_ptr)
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
2026-09-16 4:32 [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() Haren Myneni
@ 2026-09-16 4:32 ` Haren Myneni
2026-09-16 4:47 ` sashiko-bot
2026-09-16 21:33 ` Rob Herring
2026-09-16 4:43 ` [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() sashiko-bot
1 sibling, 2 replies; 8+ messages in thread
From: Haren Myneni @ 2026-09-16 4:32 UTC (permalink / raw)
To: linuxppc-dev
Cc: devicetree, maddy, robh, mpe, npiggin, saravanak, ritesh.list,
hbabu, haren
For CPU ADD, the device tree entries are retrieved with
configure-connector RTAS call and attached to the device tree.
Then the CPU is added as part of DT node notification. If the CPU
ADD notifier returns failure, the corresponding CPU node entries
should be deleted from the device-tree. See pseries_add_processor()
for the possible failure cases.
The current code does not remove CPU node entries during CPU ADD
notifier failure and causing the following issues:
- Can not add this CPU later since already present in the
device-tree.
- Trying to free memory allocated to CPU node without detaching
the node and it causes freeing its sibling node (existing CPU
nodes).
This patch fixes this issue by calling of_detach_node_no_notify()
for the failure from CPU ADD notifier which removes CPU node
entries without calling CPU REMOVE notifier.
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
arch/powerpc/platforms/pseries/dlpar.c | 9 ++++++---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 4 ++--
arch/powerpc/platforms/pseries/mobility.c | 2 +-
arch/powerpc/platforms/pseries/pmem.c | 2 +-
arch/powerpc/platforms/pseries/pseries.h | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index f4d33b8dffd8..01a9849773cd 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -247,15 +247,18 @@ int dlpar_attach_node(struct device_node *dn, struct device_node *parent)
return 0;
}
-int dlpar_detach_node(struct device_node *dn)
+int dlpar_detach_node(struct device_node *dn, bool notify)
{
struct device_node *child;
int rc;
for_each_child_of_node(dn, child)
- dlpar_detach_node(child);
+ dlpar_detach_node(child, notify);
- rc = of_detach_node(dn);
+ if (notify)
+ rc = of_detach_node(dn);
+ else
+ rc = of_detach_node_no_notify(dn);
if (rc)
return rc;
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index bc6926dbf148..db359fa3f922 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -603,7 +603,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
rc = dlpar_release_drc(drc_index);
if (!rc)
- dlpar_free_cc_nodes(dn);
+ dlpar_detach_node(dn, false);
return saved_rc;
}
@@ -616,7 +616,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
pr_warn("Failed to online cpu %pOFn, rc: %d, drc index: %x\n",
dn, rc, drc_index);
- rc = dlpar_detach_node(dn);
+ rc = dlpar_detach_node(dn, true);
if (!rc)
dlpar_release_drc(drc_index);
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index b5c2abd12432..e0e4ef04c679 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -112,7 +112,7 @@ static int delete_dt_node(struct device_node *dn)
}
pr_debug("removing node %pOFfp\n", dn);
- dlpar_detach_node(dn);
+ dlpar_detach_node(dn, true);
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pmem.c b/arch/powerpc/platforms/pseries/pmem.c
index 0f1d45f32e4a..b60a01745137 100644
--- a/arch/powerpc/platforms/pseries/pmem.c
+++ b/arch/powerpc/platforms/pseries/pmem.c
@@ -85,7 +85,7 @@ static ssize_t pmem_drc_remove_node(u32 drc_index)
pr_debug("Attempting to remove %pOF, drc index: %x\n", dn, drc_index);
/* * NB: tears down the ibm,pmemory device as a side-effect */
- rc = dlpar_detach_node(dn);
+ rc = dlpar_detach_node(dn, true);
if (rc)
return rc;
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 3968a6970fa8..6f80e58e6186 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -50,7 +50,7 @@ extern void dlpar_free_cc_property(struct property *);
extern struct device_node *dlpar_configure_connector(__be32,
struct device_node *);
extern int dlpar_attach_node(struct device_node *, struct device_node *);
-extern int dlpar_detach_node(struct device_node *);
+extern int dlpar_detach_node(struct device_node *, bool);
extern int dlpar_acquire_drc(u32 drc_index);
extern int dlpar_release_drc(u32 drc_index);
extern int dlpar_unisolate_drc(u32 drc_index);
--
2.55.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drivers/of: Add of_detach_node_no_notify()
2026-09-16 4:32 [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() Haren Myneni
2026-09-16 4:32 ` [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier Haren Myneni
@ 2026-09-16 4:43 ` sashiko-bot
2026-09-17 4:11 ` Haren Myneni
1 sibling, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-09-16 4:43 UTC (permalink / raw)
To: Haren Myneni; +Cc: devicetree, robh, conor+dt
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.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916043249.2062676-1-haren@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
2026-09-16 4:32 ` [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier Haren Myneni
@ 2026-09-16 4:47 ` sashiko-bot
2026-09-17 3:57 ` Haren Myneni
2026-09-16 21:33 ` Rob Herring
1 sibling, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-09-16 4:47 UTC (permalink / raw)
To: Haren Myneni; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] of: kernel panic in __of_detach_node() due to unattached node
- [High] powerpc/pseries: sibling device nodes leaked on hotplug error path
--
commit 4b977223f26c877c63148563cad8a84f1583e398
Author: Haren Myneni <haren@linux.ibm.com>
powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
For CPU ADD, the device tree entries are retrieved with
configure-connector RTAS call and attached to the device tree.
Then the CPU is added as part of DT node notification. If the CPU
ADD notifier returns failure, this patch calls of_detach_node_no_notify()
instead of dlpar_free_cc_nodes() to prevent device tree issues.
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index f4d33b8dffd85..01a9849773cd1 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -247,15 +247,18 @@ int dlpar_attach_node(struct device_node *dn, struct device_node *parent)
> return 0;
> }
>
> -int dlpar_detach_node(struct device_node *dn)
> +int dlpar_detach_node(struct device_node *dn, bool notify)
> {
> struct device_node *child;
> int rc;
>
> for_each_child_of_node(dn, child)
> - dlpar_detach_node(child);
> + dlpar_detach_node(child, notify);
[Severity: High]
Does this code leak sibling device nodes?
In the error handling path of dlpar_cpu_add(), when
pseries_cpuhp_attach_nodes() fails, dlpar_detach_node(dn, false) is called
to clean up. However, the old dlpar_free_cc_nodes() used to recursively
free dn->sibling. This new implementation only iterates through children and
completely ignores dn->sibling.
If siblings were returned by configure-connector but never successfully
attached, or if they were attached but later notification fails, are those
sibling nodes permanently leaked?
>
> - rc = of_detach_node(dn);
> + if (notify)
> + rc = of_detach_node(dn);
> + else
> + rc = of_detach_node_no_notify(dn);
[Severity: High]
Can this cause a NULL pointer dereference leading to a kernel panic?
If of_changeset_attach_node() fails in pseries_cpuhp_attach_nodes() (for
instance, due to an allocation failure), the dn node is never attached to
the device tree's sibling list, and its OF_DETACHED flag remains 0.
If dlpar_cpu_add() then attempts to clean up by calling
dlpar_detach_node(dn, false), this code forwards the unattached node to
of_detach_node_no_notify(dn). Since OF_DETACHED is 0, it calls
__of_detach_node(np).
Inside __of_detach_node(), the node is not found in parent->child, causing
the prevsib pointer to traverse past the end of the list to NULL, leading to
a crash at prevsib->sibling = np->sibling.
> if (rc)
> return rc;
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index bc6926dbf1489..db359fa3f9224 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -603,7 +603,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
>
> rc = dlpar_release_drc(drc_index);
> if (!rc)
> - dlpar_free_cc_nodes(dn);
> + dlpar_detach_node(dn, false);
>
> return saved_rc;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916043249.2062676-1-haren@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
2026-09-16 4:32 ` [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier Haren Myneni
2026-09-16 4:47 ` sashiko-bot
@ 2026-09-16 21:33 ` Rob Herring
2026-09-17 3:42 ` Haren Myneni
1 sibling, 1 reply; 8+ messages in thread
From: Rob Herring @ 2026-09-16 21:33 UTC (permalink / raw)
To: Haren Myneni
Cc: linuxppc-dev, devicetree, maddy, mpe, npiggin, saravanak,
ritesh.list, hbabu
On Tue, Sep 15, 2026 at 09:32:49PM -0700, Haren Myneni wrote:
> For CPU ADD, the device tree entries are retrieved with
> configure-connector RTAS call and attached to the device tree.
> Then the CPU is added as part of DT node notification. If the CPU
> ADD notifier returns failure, the corresponding CPU node entries
> should be deleted from the device-tree. See pseries_add_processor()
> for the possible failure cases.
>
> The current code does not remove CPU node entries during CPU ADD
> notifier failure and causing the following issues:
> - Can not add this CPU later since already present in the
> device-tree.
> - Trying to free memory allocated to CPU node without detaching
> the node and it causes freeing its sibling node (existing CPU
> nodes).
>
> This patch fixes this issue by calling of_detach_node_no_notify()
> for the failure from CPU ADD notifier which removes CPU node
> entries without calling CPU REMOVE notifier.
Would using the of_changeset_ API directly solve your issue? It's better
designed for handling reverting a changeset. The of_attach_node/
of_detach_node() APIs have limited users and I'd really like to remove
them. Or make them PPC specific perhaps. Looks like there is 1 non-PPC
user that snuck in.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
2026-09-16 21:33 ` Rob Herring
@ 2026-09-17 3:42 ` Haren Myneni
0 siblings, 0 replies; 8+ messages in thread
From: Haren Myneni @ 2026-09-17 3:42 UTC (permalink / raw)
To: Rob Herring
Cc: linuxppc-dev, devicetree, maddy, mpe, npiggin, saravanak,
ritesh.list, tyreld
On Wed, 2026-09-16 at 16:33 -0500, Rob Herring wrote:
> On Tue, Sep 15, 2026 at 09:32:49PM -0700, Haren Myneni wrote:
> > For CPU ADD, the device tree entries are retrieved with
> > configure-connector RTAS call and attached to the device tree.
> > Then the CPU is added as part of DT node notification. If the CPU
> > ADD notifier returns failure, the corresponding CPU node entries
> > should be deleted from the device-tree. See pseries_add_processor()
> > for the possible failure cases.
> >
> > The current code does not remove CPU node entries during CPU ADD
> > notifier failure and causing the following issues:
> > - Can not add this CPU later since already present in the
> > device-tree.
> > - Trying to free memory allocated to CPU node without detaching
> > the node and it causes freeing its sibling node (existing CPU
> > nodes).
> >
> > This patch fixes this issue by calling of_detach_node_no_notify()
> > for the failure from CPU ADD notifier which removes CPU node
> > entries without calling CPU REMOVE notifier.
>
> Would using the of_changeset_ API directly solve your issue? It's
> better
> designed for handling reverting a changeset. The of_attach_node/
> of_detach_node() APIs have limited users and I'd really like to
> remove
> them. Or make them PPC specific perhaps. Looks like there is 1 non-
> PPC
> user that snuck in.
Thanks for your comments.
of_changeset_apply(): After adding all entries, calls notifiers for
each node and continue even after one notifier failure and returns
failure. The caller does not know whether the failure is from attach
node or notifier and from which node. So
__of_changeset_revert_entries() can be used to revert entries if
stopped on the first failure.
of_changeset_revert(): always calls notifier for each node. I can use
__of_changeset_revert_entries() since CPU add has just one notifier but
not a standard interface.
pseries_cpuhp_attach_nodes():
if (!of_node_check_flag(np, OF_DETACHED))
__of_changeset_revert_entries(&cs, *ret);
The current code calls dlpar_detach_node() if online CPU failed after
success from of_changeset_apply(). Hence added
of_detach_node_no_notify(). PPC code calls of_changeset_apply() for
multiple nodes and of_attach_node/of_detach_node for single node.
Or defining some other node flag OF_NOTIFIED to call the notifier for
revert / detach in of_changeset_*.
Please let me know if you prefer to change of_changeset_*() instead of
of_detach_node_no_notify(0.
- Haren
>
>
> Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier
2026-09-16 4:47 ` sashiko-bot
@ 2026-09-17 3:57 ` Haren Myneni
0 siblings, 0 replies; 8+ messages in thread
From: Haren Myneni @ 2026-09-17 3:57 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, conor+dt, robh, tyreld
On Wed, 2026-09-16 at 04:47 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> - [High] of: kernel panic in __of_detach_node() due to unattached
> node
> - [High] powerpc/pseries: sibling device nodes leaked on hotplug
> error path
> --
>
> commit 4b977223f26c877c63148563cad8a84f1583e398
> Author: Haren Myneni <haren@linux.ibm.com>
>
> powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD
> notifier
>
> For CPU ADD, the device tree entries are retrieved with
> configure-connector RTAS call and attached to the device tree.
> Then the CPU is added as part of DT node notification. If the CPU
> ADD notifier returns failure, this patch calls
> of_detach_node_no_notify()
> instead of dlpar_free_cc_nodes() to prevent device tree issues.
>
> > diff --git a/arch/powerpc/platforms/pseries/dlpar.c
> > b/arch/powerpc/platforms/pseries/dlpar.c
> > index f4d33b8dffd85..01a9849773cd1 100644
> > --- a/arch/powerpc/platforms/pseries/dlpar.c
> > +++ b/arch/powerpc/platforms/pseries/dlpar.c
> > @@ -247,15 +247,18 @@ int dlpar_attach_node(struct device_node *dn,
> > struct device_node *parent)
> > return 0;
> > }
> >
> > -int dlpar_detach_node(struct device_node *dn)
> > +int dlpar_detach_node(struct device_node *dn, bool notify)
> > {
> > struct device_node *child;
> > int rc;
> >
> > for_each_child_of_node(dn, child)
> > - dlpar_detach_node(child);
> > + dlpar_detach_node(child, notify);
>
> [Severity: High]
> Does this code leak sibling device nodes?
>
> In the error handling path of dlpar_cpu_add(), when
> pseries_cpuhp_attach_nodes() fails, dlpar_detach_node(dn, false) is
> called
> to clean up. However, the old dlpar_free_cc_nodes() used to
> recursively
> free dn->sibling. This new implementation only iterates through
> children and
> completely ignores dn->sibling.
>
> If siblings were returned by configure-connector but never
> successfully
> attached, or if they were attached but later notification fails, are
> those
> sibling nodes permanently leaked?
of_attach_node() sets the one of current CPU nodes as sibling to the
new CPU node.
Since the sibling nodes are the existing CPU nodes, should not be
freed.
The current code (dlpar_free_cc_nodes(dn)) removes sibling nodes which
is a bug and causes an issue when trying to add new CPU in future..
>
> >
> > - rc = of_detach_node(dn);
> > + if (notify)
> > + rc = of_detach_node(dn);
> > + else
> > + rc = of_detach_node_no_notify(dn);
>
> [Severity: High]
> Can this cause a NULL pointer dereference leading to a kernel panic?
>
> If of_changeset_attach_node() fails in pseries_cpuhp_attach_nodes()
> (for
> instance, due to an allocation failure), the dn node is never
> attached to
> the device tree's sibling list, and its OF_DETACHED flag remains 0.
>
> If dlpar_cpu_add() then attempts to clean up by calling
> dlpar_detach_node(dn, false), this code forwards the unattached node
> to
> of_detach_node_no_notify(dn). Since OF_DETACHED is 0, it calls
> __of_detach_node(np).
>
> Inside __of_detach_node(), the node is not found in parent->child,
> causing
> the prevsib pointer to traverse past the end of the list to NULL,
> leading to
> a crash at prevsib->sibling = np->sibling.
will change:
if (rc && dn) /* in case of memory allocation failure */
dlpar_detach_node(dn, false);
>
> > if (rc)
> > return rc;
> >
> > diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > index bc6926dbf1489..db359fa3f9224 100644
> > --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> > @@ -603,7 +603,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> >
> > rc = dlpar_release_drc(drc_index);
> > if (!rc)
> > - dlpar_free_cc_nodes(dn);
> > + dlpar_detach_node(dn, false);
> >
> > return saved_rc;
> > }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drivers/of: Add of_detach_node_no_notify()
2026-09-16 4:43 ` [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() sashiko-bot
@ 2026-09-17 4:11 ` Haren Myneni
0 siblings, 0 replies; 8+ messages in thread
From: Haren Myneni @ 2026-09-17 4:11 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt, tyreld
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-17 4:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 4:32 [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() Haren Myneni
2026-09-16 4:32 ` [PATCH 2/2] powerpc/pseries/dlpar: Remove DT entries if failure from CPU ADD notifier Haren Myneni
2026-09-16 4:47 ` sashiko-bot
2026-09-17 3:57 ` Haren Myneni
2026-09-16 21:33 ` Rob Herring
2026-09-17 3:42 ` Haren Myneni
2026-09-16 4:43 ` [PATCH 1/2] drivers/of: Add of_detach_node_no_notify() sashiko-bot
2026-09-17 4:11 ` Haren Myneni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox