[PATCH 0/4] software node: fix lookup of swnode in release path

[PATCH 0/4] software node: fix lookup of swnode in release path

[Peng Fan (OSS)]

This series fixes a software node lifetime bug where a lookup can find an
swnode whose kobject refcount has already reached zero.

The kset list keeps an swnode linked until kobject_cleanup() calls
kobj_kset_leave(), which happens after the last kobject_put() has already
dropped the refcount to zero. A lookup that does not check whether the
object is still referenceable can therefore call kobject_get() on a dead
object, triggering:

  refcount_t: addition on 0; use-after-free
  refcount_t: saturated; leaking memory
  refcount_t: underflow; use-after-free

The problem is easily reproduced on i.MX95 with dwc3-imx when
dwc3_core_probe() returns -EPROBE_DEFER. dwc3_imx_probe() removes its
static software node from the device and the deferred probe retry
immediately adds the same node again. With
CONFIG_DEBUG_KOBJECT_RELEASE the old swnode stays on the kset list for
some time after its refcount reaches zero, so the retry reliably finds
it.

The first patch fixes the pointer-based kset lookup by using
kobject_get_unless_zero() and skipping entries that are already in their
release path. It also makes registration return the newly created swnode
reference so that device_add_software_node() does not have to look the
node up again after registering it. Reference-owning callers get a
reference from the lookup, while software_node_fwnode() keeps its
borrowed-pointer semantics by dropping the temporary lookup reference.

The remaining patches harden related weak lookups for the same class of
problem. The series is organized as follows:

  1. Fix the pointer-based kset lookup and avoid the second lookup after
     registration.
  2. Harden software_node_find_by_name() against dying entries.
  3. Check the result of software_node_fwnode() in the graph
     remote-endpoint path.
  4. Skip dying children when walking the parent's children list.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Peng Fan (4):
      software node: do not resurrect swnode being released
      software node: use kobject_get_unless_zero() in software_node_find_by_name()
      software node: handle missing remote endpoint node
      software node: skip dying children during traversal

 drivers/base/swnode.c | 143 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 100 insertions(+), 43 deletions(-)
---
base-commit: 2637bdec0535a2044cd830eba9a79363651d9043
change-id: 20261104-swnode-race-v1-b9f2dcc954c8

Best regards,
--  
Peng Fan <peng.fan@nxp.com>

[PATCH 1/4] software node: do not resurrect swnode being released

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

software_node_to_swnode() walks the kset list under list_lock, but the
last kobject_put() drops the reference count to zero before
kobject_cleanup() removes the object from the list. A lookup can
therefore find a zero-ref swnode and call kobject_get() on it, which
triggers refcount warnings and a use-after-free.

One way to hit this is the dwc3-imx driver on i.MX95: when
dwc3_core_probe() returns -EPROBE_DEFER, dwc3_imx_probe() removes the
static software node from the device, and the deferred probe retry
immediately adds the same node again. The previous swnode is still
waiting for its release callback, and CONFIG_DEBUG_KOBJECT_RELEASE makes
the window large and deterministic.

Fix the lookup by acquiring references with kobject_get_unless_zero()
and skipping objects that are already in their release path. Use a
reference-returning variant for callers that keep the node (register,
unregister and device_add_software_node()) and keep a borrowed-pointer
variant for software_node_fwnode().

To avoid looking the node up again after registration,
software_node_register() now uses an internal helper that returns the
newly created swnode reference, and device_add_software_node() consumes
that reference directly.

Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/swnode.c | 114 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 78 insertions(+), 36 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 84a77ffa3dc4b..19a4153f3fac1 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -75,8 +75,28 @@ static inline struct swnode *dev_to_swnode(struct device *dev)
 	return to_swnode(fwnode);
 }
 
+static struct swnode *swnode_get(struct swnode *swnode)
+{
+	kobject_get(&swnode->kobj);
+
+	return swnode;
+}
+
+static void swnode_put(struct swnode *swnode)
+{
+	kobject_put(&swnode->kobj);
+}
+
+/*
+ * Find the swnode registered for @node and return it with a new reference.
+ *
+ * A kobject remains on the kset list until its release callback runs, so an
+ * entry whose last reference has already been dropped can still be found
+ * here.  Use kobject_get_unless_zero() to skip such objects instead of
+ * resurrecting them.
+ */
 static struct swnode *
-software_node_to_swnode(const struct software_node *node)
+software_node_to_swnode_get(const struct software_node *node)
 {
 	struct swnode *swnode = NULL;
 	struct kobject *k;
@@ -87,10 +107,16 @@ software_node_to_swnode(const struct software_node *node)
 	spin_lock(&swnode_kset->list_lock);
 
 	list_for_each_entry(k, &swnode_kset->list, entry) {
-		swnode = kobj_to_swnode(k);
-		if (swnode->node == node)
-			break;
-		swnode = NULL;
+		struct swnode *candidate = kobj_to_swnode(k);
+
+		if (candidate->node != node)
+			continue;
+
+		if (!kobject_get_unless_zero(k))
+			continue;
+
+		swnode = candidate;
+		break;
 	}
 
 	spin_unlock(&swnode_kset->list_lock);
@@ -98,6 +124,21 @@ software_node_to_swnode(const struct software_node *node)
 	return swnode;
 }
 
+/*
+ * Borrowed-pointer variant of software_node_to_swnode_get().  The caller must
+ * guarantee the lifetime of @node by other means.
+ */
+static struct swnode *
+software_node_to_swnode(const struct software_node *node)
+{
+	struct swnode *swnode = software_node_to_swnode_get(node);
+
+	if (swnode)
+		swnode_put(swnode);
+
+	return swnode;
+}
+
 const struct software_node *to_software_node(const struct fwnode_handle *fwnode)
 {
 	const struct swnode *swnode = to_swnode(fwnode);
@@ -374,18 +415,6 @@ EXPORT_SYMBOL_GPL(property_entries_free);
 /* -------------------------------------------------------------------------- */
 /* fwnode operations */
 
-static struct swnode *swnode_get(struct swnode *swnode)
-{
-	kobject_get(&swnode->kobj);
-
-	return swnode;
-}
-
-static void swnode_put(struct swnode *swnode)
-{
-	kobject_put(&swnode->kobj);
-}
-
 static struct fwnode_handle *software_node_get(struct fwnode_handle *fwnode)
 {
 	struct swnode *swnode = swnode_get(to_swnode(fwnode));
@@ -977,21 +1006,39 @@ void software_node_unregister_node_group(const struct software_node * const *nod
 }
 EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
 
+static struct swnode *
+software_node_register_node(const struct software_node *node)
+{
+	struct swnode *parent, *swnode;
+	struct fwnode_handle *fwnode;
+
+	swnode = software_node_to_swnode_get(node);
+	if (swnode) {
+		swnode_put(swnode);
+		return ERR_PTR(-EEXIST);
+	}
+
+	parent = software_node_to_swnode_get(node->parent);
+	if (node->parent && !parent)
+		return ERR_PTR(-EINVAL);
+
+	fwnode = swnode_register(node, parent, 0);
+	if (parent)
+		swnode_put(parent);
+
+	if (IS_ERR(fwnode))
+		return ERR_CAST(fwnode);
+
+	return to_swnode(fwnode);
+}
+
 /**
  * software_node_register - Register static software node
  * @node: The software node to be registered
  */
 int software_node_register(const struct software_node *node)
 {
-	struct swnode *parent = software_node_to_swnode(node->parent);
-
-	if (software_node_to_swnode(node))
-		return -EEXIST;
-
-	if (node->parent && !parent)
-		return -EINVAL;
-
-	return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0));
+	return PTR_ERR_OR_ZERO(software_node_register_node(node));
 }
 EXPORT_SYMBOL_GPL(software_node_register);
 
@@ -1061,21 +1108,16 @@ EXPORT_SYMBOL_GPL(fwnode_remove_software_node);
 int device_add_software_node(struct device *dev, const struct software_node *node)
 {
 	struct swnode *swnode;
-	int ret;
 
 	/* Only one software node per device. */
 	if (dev_to_swnode(dev))
 		return -EBUSY;
 
-	swnode = software_node_to_swnode(node);
-	if (swnode) {
-		swnode_get(swnode);
-	} else {
-		ret = software_node_register(node);
-		if (ret)
-			return ret;
-
-		swnode = software_node_to_swnode(node);
+	swnode = software_node_to_swnode_get(node);
+	if (!swnode) {
+		swnode = software_node_register_node(node);
+		if (IS_ERR(swnode))
+			return PTR_ERR(swnode);
 	}
 
 	set_secondary_fwnode(dev, &swnode->fwnode);

-- 
2.34.1

[PATCH 2/4] software node: use kobject_get_unless_zero() in software_node_find_by_name()

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

software_node_find_by_name() also walks the swnode kset list and takes a
reference with swnode_get(). Like the pointer-based lookup, it can
encounter an entry whose refcount has already reached zero but which is
still linked until its delayed release callback runs.

Use kobject_get_unless_zero() and continue scanning when the entry is
already dying.

Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/swnode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 19a4153f3fac1..9095d50ba146f 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -831,8 +831,8 @@ software_node_find_by_name(const struct software_node *parent, const char *name)
 		swnode = kobj_to_swnode(k);
 		if (parent == swnode->node->parent && swnode->node->name &&
 		    !strcmp(name, swnode->node->name)) {
-			swnode_get(swnode);
-			break;
+			if (kobject_get_unless_zero(k))
+				break;
 		}
 		swnode = NULL;
 	}

-- 
2.34.1

[PATCH 3/4] software node: handle missing remote endpoint node

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

software_node_graph_get_remote_endpoint() passes the result of
software_node_fwnode() directly to software_node_get(). That lookup
returns NULL if the referenced software node is not registered, and now
also if it is in its release path.

Check the result and return NULL instead of passing a NULL fwnode to
software_node_get().

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/swnode.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 9095d50ba146f..4d95a59a27f0a 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -676,6 +676,7 @@ software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
 {
 	struct swnode *swnode = to_swnode(fwnode);
 	const struct software_node_ref_args *ref;
+	struct fwnode_handle *refnode;
 	const struct property_entry *prop;
 
 	if (!swnode)
@@ -690,7 +691,11 @@ software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
 	if (!ref->swnode)
 		return NULL;
 
-	return software_node_get(software_node_fwnode(ref->swnode));
+	refnode = software_node_fwnode(ref->swnode);
+	if (!refnode)
+		return NULL;
+
+	return software_node_get(refnode);
 }
 
 static struct fwnode_handle *

-- 
2.34.1

[PATCH 4/4] software node: skip dying children during traversal

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

A child swnode is removed from its parent's children list in
software_node_release(), which runs after its kobject refcount has
already reached zero. The fwnode child traversal helpers can therefore
find an entry that is in its release path and call kobject_get() on a
zero refcount.

Use kobject_get_unless_zero() in software_node_get_next_child() and
software_node_get_named_child_node() and keep scanning until a live
child is found or the list ends.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/base/swnode.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 4d95a59a27f0a..1354447069e10 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -515,7 +515,18 @@ software_node_get_next_child(const struct fwnode_handle *fwnode,
 		c = list_first_entry(&p->children, struct swnode, entry);
 
 	fwnode_handle_put(child);
-	return fwnode_handle_get(&c->fwnode);
+
+	for (;;) {
+		if (kobject_get_unless_zero(&c->kobj))
+			return &c->fwnode;
+
+		if (list_is_last(&c->entry, &p->children))
+			break;
+
+		c = list_next_entry(c, entry);
+	}
+
+	return NULL;
 }
 
 static struct fwnode_handle *
@@ -529,10 +540,9 @@ software_node_get_named_child_node(const struct fwnode_handle *fwnode,
 		return NULL;
 
 	list_for_each_entry(child, &swnode->children, entry) {
-		if (!strcmp(childname, kobject_name(&child->kobj))) {
-			swnode_get(child);
+		if (!strcmp(childname, kobject_name(&child->kobj)) &&
+		    kobject_get_unless_zero(&child->kobj))
 			return &child->fwnode;
-		}
 	}
 	return NULL;
 }

-- 
2.34.1

Re: [PATCH 1/4] software node: do not resurrect swnode being released

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] software node: transient ref in software_node_to_swnode() risks UAF
- [Medium] software node: re-registering named or parent node hits -EEXIST
--

commit d02f059d143a3e75e357a6d906b17afb37852f09
Author: Peng Fan <peng.fan@nxp.com>

software node: do not resurrect swnode being released

This commit updates software node lookup in drivers/base/swnode.c to use
kobject_get_unless_zero() so that zero-reference swnodes awaiting release on
swnode_kset->list are skipped rather than resurrected. It introduces
software_node_to_swnode_get() and software_node_register_node() for callers
that acquire a reference, while keeping software_node_to_swnode() as a
wrapper that drops the reference before returning.

>     software node: do not resurrect swnode being released
>     
>     software_node_to_swnode() walks the kset list under list_lock, but the
>     last kobject_put() drops the reference count to zero before
>     kobject_cleanup() removes the object from the list. A lookup can
>     therefore find a zero-ref swnode and call kobject_get() on it, which
>     triggers refcount warnings and a use-after-free.
>     
>     One way to hit this is the dwc3-imx driver on i.MX95: when
>     dwc3_core_probe() returns -EPROBE_DEFER, dwc3_imx_probe() removes the
>     static software node from the device, and the deferred probe retry
>     immediately adds the same node again. The previous swnode is still
>     waiting for its release callback, and CONFIG_DEBUG_KOBJECT_RELEASE makes
>     the window large and deterministic.
>     
>     Fix the lookup by acquiring references with kobject_get_unless_zero()
>     and skipping objects that are already in their release path. Use a
>     reference-returning variant for callers that keep the node (register,
>     unregister and device_add_software_node()) and keep a borrowed-pointer
>     variant for software_node_fwnode().
>     
>     To avoid looking the node up again after registration,
>     software_node_register() now uses an internal helper that returns the
>     newly created swnode reference, and device_add_software_node() consumes
>     that reference directly.
>     
>     Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
>     Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 84a77ffa3dc4b..19a4153f3fac1 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c

[ ... ]

> @@ -98,6 +124,21 @@ software_node_to_swnode(const struct software_node *node)
>  	return swnode;
>  }
>  
> +/*
> + * Borrowed-pointer variant of software_node_to_swnode_get().  The caller must
> + * guarantee the lifetime of @node by other means.
> + */
> +static struct swnode *
> +software_node_to_swnode(const struct software_node *node)
> +{
> +	struct swnode *swnode = software_node_to_swnode_get(node);
> +
> +	if (swnode)
> +		swnode_put(swnode);
> +
> +	return swnode;
> +}

[Severity: High]
Can calling swnode_put(swnode) here drop the final reference and free swnode
before returning it?

In software_node_to_swnode(), software_node_to_swnode_get(node) acquires
swnode_kset->list_lock, increments candidate->kobj.kref (for example from 1
to 2) via kobject_get_unless_zero(), and releases list_lock.

If the owner of the swnode drops its final reference (via
device_remove_software_node(), software_node_unregister(), or
fwnode_remove_software_node()) before software_node_to_swnode() runs
swnode_put(swnode), the owner's kobject_put() only decrements kref from 2 to
1 instead of releasing the object:

1. If the owner immediately re-registers or re-adds the software_node during
   that window, software_node_to_swnode_get() sees the dying swnode with
   kref == 1 and either fails software_node_register() with -EEXIST or
   reattaches the dying swnode in device_add_software_node().

2. When software_node_to_swnode() then calls swnode_put(swnode), it drops
   kref from 1 to 0, invoking kobject_release() -> kobject_cleanup() ->
   software_node_release() -> kfree(swnode), and returns a dangling swnode
   pointer to its caller.

This is reachable through software_node_fwnode() when resolving referenced
supplier or remote nodes (whose lifetimes are not pinned by the caller) in
software_node_get_reference_args(), software_node_graph_get_remote_endpoint(),
and software_node_add_links(), as well as through software_node_unregister():

drivers/base/swnode.c:software_node_unregister() {
	struct swnode *swnode;

	swnode = software_node_to_swnode(node);
	if (swnode)
		fwnode_remove_software_node(&swnode->fwnode);
}

Also, the commit message mentions using the reference-returning variant for
unregister ("Use a reference-returning variant for callers that keep the node
(register, unregister and device_add_software_node())"), but
software_node_unregister() still calls software_node_to_swnode(node).

[ ... ]

> @@ -977,21 +1006,39 @@ void software_node_unregister_node_group(const struct software_node * const *nod
>  }
>  EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
>  
> +static struct swnode *
> +software_node_register_node(const struct software_node *node)
> +{
> +	struct swnode *parent, *swnode;
> +	struct fwnode_handle *fwnode;
> +
> +	swnode = software_node_to_swnode_get(node);
> +	if (swnode) {
> +		swnode_put(swnode);
> +		return ERR_PTR(-EEXIST);
> +	}
> +
> +	parent = software_node_to_swnode_get(node->parent);
> +	if (node->parent && !parent)
> +		return ERR_PTR(-EINVAL);
> +
> +	fwnode = swnode_register(node, parent, 0);

[Severity: Medium]
What happens if a named static software_node (node->name != NULL) or a
parent/child software_node group is removed on -EPROBE_DEFER and
re-registered before kobject_cleanup() runs?

When the last reference to an swnode is dropped via swnode_put() ->
kobject_put(), __kobject_del() (which removes the sysfs directory) and
kobject_put(parent) only run inside kobject_cleanup(), which is delayed by
1-4 seconds on a workqueue when CONFIG_DEBUG_KOBJECT_RELEASE=y.

For unnamed nodes (node->name == NULL, such as dwc3_imx_swnode),
swnode_register() allocates a fresh IDA ID and creates "node1" while "node0"
is still waiting to be cleaned up. However, when node->name is set:

drivers/base/swnode.c:swnode_register() {
	...
	if (node->name)
		ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
					   parent ? &parent->kobj : NULL,
					   "%s", node->name);
}

kobject_init_and_add() attempts to create a sysfs directory with the same
name as the dying swnode that is still present in sysfs, failing in
sysfs_create_dir_ns() with -EEXIST and a sysfs_warn_dup() warning.

Similarly, when a node group is unregistered via
software_node_unregister_node_group(), the dying child kobject holds its
reference on parent->kobj until kobject_cleanup() finishes; the parent's
refcount therefore stays non-zero during the delayed release window, and
re-registering the parent in software_node_register_node() still fails with
-EEXIST.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-swnode-race-v1-v1-0-9406a67e25be@nxp.com?part=1

Re: [PATCH 2/4] software node: use kobject_get_unless_zero() in software_node_find_by_name()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] software node: use-after-free on swnode->node in software_node_find_by_name()
- [Low] software node: wrong Fixes tag for software_node_find_by_name()
--

commit e919f34826aef89486e543d4a209837a0357d8a1
Author: Peng Fan <peng.fan@nxp.com>

software node: use kobject_get_unless_zero() in software_node_find_by_name()

This commit updates software_node_find_by_name() to use
kobject_get_unless_zero() instead of swnode_get() so that it skips entries in
swnode_kset->list whose reference count has already dropped to zero.

>     software node: use kobject_get_unless_zero() in software_node_find_by_name()
>     
>     software_node_find_by_name() also walks the swnode kset list and takes a
>     reference with swnode_get(). Like the pointer-based lookup, it can
>     encounter an entry whose refcount has already reached zero but which is
>     still linked until its delayed release callback runs.
>     
>     Use kobject_get_unless_zero() and continue scanning when the entry is
>     already dying.
>     
>     Fixes: 80488a6b1d3c ("software node: Add support for static node descriptors")
>     Signed-off-by: Peng Fan <peng.fan@nxp.com>

[Severity: Low]
Commit 80488a6b1d3c ("software node: Add support for static node
descriptors") introduced software_node_to_swnode(), while
software_node_find_by_name() was added later. Should the Fixes tag instead
reference the commit that introduced software_node_find_by_name()?

  Fixes: 1666faedb567 ("software node: Add software_node_find_by_name()")

> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 19a4153f3fac1..9095d50ba146f 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -831,8 +831,8 @@ software_node_find_by_name(const struct software_node *parent, const char *name)
>  		swnode = kobj_to_swnode(k);
>  		if (parent == swnode->node->parent && swnode->node->name &&
>  		    !strcmp(name, swnode->node->name)) {
> -			swnode_get(swnode);
> -			break;
> +			if (kobject_get_unless_zero(k))
> +				break;
>  		}

[Severity: High]
Can dereferencing swnode->node->parent and swnode->node->name before calling
kobject_get_unless_zero(k) cause a use-after-free when scanning past a dying
swnode?

When a software node is registered via software_node_register(),
software_node_register_node_group(), or device_add_software_node(),
swnode->allocated is 0 and swnode->node points directly to caller-owned
memory rather than an internal copy freed in software_node_release():

drivers/base/swnode.c:software_node_release() {
    ...
	if (swnode->allocated)
		software_node_free(swnode->node);
    ...
}

Once software_node_unregister() drops swnode->kobj.kref to zero and returns,
the caller can immediately free or unmap the backing struct software_node
while the dying swnode remains on swnode_kset->list awaiting kobject cleanup
(for example when CONFIG_DEBUG_KOBJECT_RELEASE delays release, or before
kset_leave() acquires swnode_kset->list_lock).

For instance, ipu_bridge_init() in drivers/media/pci/intel/ipu-bridge.c
unregisters a heap-allocated node and immediately frees it:

err_unregister_ipu:
	software_node_unregister(&bridge->ipu_hid_node);
err_free_bridge:
	kfree(bridge);

Similarly, fwnode_pointer() in lib/tests/printf_kunit.c registers
stack-allocated software_node structures and unregisters them right before
returning from the stack frame.

Should software_node_find_by_name() acquire the reference via
kobject_get_unless_zero(k) before dereferencing swnode->node, and drop that
reference if the parent or name does not match?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-swnode-race-v1-v1-0-9406a67e25be@nxp.com?part=2

Re: [PATCH 3/4] software node: handle missing remote endpoint node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] software node: refcount race in software_node_graph_get_remote_endpoint()
--

commit 248da443d964ed22789819ccc10771552e53780f
Author: Peng Fan <peng.fan@nxp.com>

software node: handle missing remote endpoint node

This commit updates software_node_graph_get_remote_endpoint() to check the
return value of software_node_fwnode() for NULL before passing it to
software_node_get().

> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 9095d50ba146f..4d95a59a27f0a 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -676,6 +676,7 @@ software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
>  {
>  	struct swnode *swnode = to_swnode(fwnode);
>  	const struct software_node_ref_args *ref;
> +	struct fwnode_handle *refnode;
>  	const struct property_entry *prop;
>  
>  	if (!swnode)
> @@ -690,7 +691,11 @@ software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
>  	if (!ref->swnode)
>  		return NULL;
>  
> -	return software_node_get(software_node_fwnode(ref->swnode));
> +	refnode = software_node_fwnode(ref->swnode);
> +	if (!refnode)
> +		return NULL;
> +
> +	return software_node_get(refnode);

[Severity: High]
Can this race with a concurrent unregistration of ref->swnode and cause a
use-after-free or refcount addition on zero?

software_node_fwnode() calls software_node_to_swnode(), which acquires a
reference via software_node_to_swnode_get() and immediately drops it:

drivers/base/swnode.c:software_node_to_swnode() {
	struct swnode *swnode = software_node_to_swnode_get(node);

	if (swnode)
		swnode_put(swnode);

	return swnode;
}

Because the caller of software_node_graph_get_remote_endpoint() only holds
a reference to the local endpoint fwnode and not to ref->swnode, if
ref->swnode is concurrently unregistered after kobject_get_unless_zero()
succeeds in software_node_to_swnode_get(), either swnode_put() inside
software_node_to_swnode() or the concurrent unregistration can drop the
last reference before software_node_get(refnode) runs.

software_node_get(refnode) will then call kobject_get() on a dying or
freed swnode.

Should software_node_graph_get_remote_endpoint() call
software_node_to_swnode_get(ref->swnode) directly and return
&remote->fwnode (or NULL) so that the reference acquired under
swnode_kset->list_lock via kobject_get_unless_zero() is retained?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-swnode-race-v1-v1-0-9406a67e25be@nxp.com?part=3

Re: [PATCH 4/4] software node: skip dying children during traversal

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] software node: unsynchronized child list traversal races with release
--

commit 9feb88166afe18788cf51497c2968b7148e1d347
Author: Peng Fan <peng.fan@nxp.com>

software node: skip dying children during traversal

This commit updates software_node_get_next_child() and
software_node_get_named_child_node() to use kobject_get_unless_zero() so
that child swnodes whose kobject refcount has reached zero are skipped
during child list traversal.

> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 4d95a59a27f0a..1354447069e10 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -515,7 +515,18 @@ software_node_get_next_child(const struct fwnode_handle *fwnode,
>  		c = list_first_entry(&p->children, struct swnode, entry);
>  
>  	fwnode_handle_put(child);
> -	return fwnode_handle_get(&c->fwnode);
> +
> +	for (;;) {
> +		if (kobject_get_unless_zero(&c->kobj))
> +			return &c->fwnode;
> +
> +		if (list_is_last(&c->entry, &p->children))
> +			break;
> +
> +		c = list_next_entry(c, entry);
> +	}
> +
> +	return NULL;
>  }
>  
>  static struct fwnode_handle *
> @@ -529,10 +540,9 @@ software_node_get_named_child_node(const struct fwnode_handle *fwnode,
>  		return NULL;
>  
>  	list_for_each_entry(child, &swnode->children, entry) {
> -		if (!strcmp(childname, kobject_name(&child->kobj))) {
> -			swnode_get(child);
> +		if (!strcmp(childname, kobject_name(&child->kobj)) &&
> +		    kobject_get_unless_zero(&child->kobj))
>  			return &child->fwnode;

[Severity: High]
Can walking the parent's children list and skipping zero-refcount nodes here
race with software_node_release() and trigger a use-after-free?

Unlike swnode_kset->list, which is protected by swnode_kset->list_lock in
software_node_to_swnode_get() and software_node_find_by_name() and unlinked
under that same lock in kobj_kset_leave() before software_node_release()
runs, parent->children has no lock serializing list_add_tail() in
swnode_register(), list_del() and kfree() in software_node_release(), or the
traversal in software_node_get_next_child() and
software_node_get_named_child_node():

drivers/base/swnode.c:software_node_release() {
	struct swnode *swnode = kobj_to_swnode(kobj);

	if (swnode->parent) {
		ida_free(&swnode->parent->child_ids, swnode->id);
		list_del(&swnode->entry);
	} else {
		ida_free(&swnode_root_ids, swnode->id);
	}
	...
	kfree(swnode);
}

In software_node_get_next_child(), when kobject_get_unless_zero(&c->kobj)
returns NULL because c's refcount has already dropped to zero, nothing pins
c in memory or on p->children. A concurrent software_node_release() can run
list_del(&c->entry) (poisoning c->entry.next with LIST_POISON1) and kfree(c)
before the loop checks list_is_last(&c->entry, &p->children) and advances
via list_next_entry(c, entry), or even right after list_first_entry() or
list_next_entry() loads c.

Similarly, in software_node_get_named_child_node(), list_for_each_entry()
walks swnode->children without any lock and dereferences
kobject_name(&child->kobj) before calling kobject_get_unless_zero(), while
software_node_release() may be concurrently unlinking and freeing child.

Does traversal and modification of the parent's children list (and child_ids
in software_node_release()) need a lock to synchronize against concurrent
child release?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-swnode-race-v1-v1-0-9406a67e25be@nxp.com?part=4

Re: [PATCH 2/4] software node: use kobject_get_unless_zero() in software_node_find_by_name()

[Andy Shevchenko]

On Wed, Nov 04, 2026 at 11:43:14PM +0800, Peng Fan (OSS) wrote:

> software_node_find_by_name() also walks the swnode kset list and takes a
> reference with swnode_get(). Like the pointer-based lookup, it can
> encounter an entry whose refcount has already reached zero but which is
> still linked until its delayed release callback runs.
> 
> Use kobject_get_unless_zero() and continue scanning when the entry is
> already dying.

...

>  		if (parent == swnode->node->parent && swnode->node->name &&
>  		    !strcmp(name, swnode->node->name)) {
> -			swnode_get(swnode);
> -			break;
> +			if (kobject_get_unless_zero(k))
> +				break;

Can we rather see even in the previous patch something like

... swnode_get_alive(...)
{
	...uses kobject_get_unless_zero()...
}

?

>  		}

-- 
With Best Regards,
Andy Shevchenko