[PATCH] component: Release match data on add failure

[PATCH] component: Release match data on add failure

[Ruoyu Wang]

component_match_add_release() accepts a release callback so callers can
transfer ownership of resources held by the match data. However,
__component_match_add() returns without invoking that callback when the
match is already an error pointer or when allocating the match or its
array fails.

This leaks resources acquired before the call. For example,
drm_of_component_match_add() takes a reference to its device node before
adding the match, and ERR_PTR-aware callers can return from probe with
that reference still held. Calls made after the first allocation failure
can leak further references.

Invoke the release callback whenever the match data cannot be added, and
document the failure-path ownership rule. Successful match lifetime
semantics remain unchanged.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: ce657b1cddf1 ("component: add support for releasing match data")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/base/component.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 655d68deb590c..2b455d7499926 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -388,14 +388,14 @@ static void __component_match_add(struct device *parent,
 	struct component_match *match = *matchptr;
 
 	if (IS_ERR(match))
-		return;
+		goto err_release;
 
 	if (!match) {
 		match = devres_alloc(devm_component_match_release,
 				     sizeof(*match), GFP_KERNEL);
 		if (!match) {
 			*matchptr = ERR_PTR(-ENOMEM);
-			return;
+			goto err_release;
 		}
 
 		devres_add(parent, match);
@@ -410,7 +410,7 @@ static void __component_match_add(struct device *parent,
 		ret = component_match_realloc(match, new_size);
 		if (ret) {
 			*matchptr = ERR_PTR(ret);
-			return;
+			goto err_release;
 		}
 	}
 
@@ -420,6 +420,11 @@ static void __component_match_add(struct device *parent,
 	match->compare[match->num].data = compare_data;
 	match->compare[match->num].component = NULL;
 	match->num++;
+	return;
+
+err_release:
+	if (release)
+		release(parent, compare_data);
 }
 
 /**
@@ -438,7 +443,8 @@ static void __component_match_add(struct device *parent,
  * The allocated match list in @matchptr is automatically released using devm
  * actions, where upon @release will be called to free any references held by
  * @compare_data, e.g. when @compare_data is a &device_node that must be
- * released with of_node_put().
+ * released with of_node_put(). @release is also called if the match cannot be
+ * added.
  *
  * See also component_match_add() and component_match_add_typed().
  */
-- 
2.51.0

Re: [PATCH] component: Release match data on add failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `component_master_add_with_match()` blindly dereferences the `match` parameter without checking if it is an `ERR_PTR`, leading to a kernel panic.
--

--- Patch [1]: [PATCH] component: Release match data on add failure ---
commit c4097bf086b1fb54cc12acc0308e520336458718
Author: Ruoyu Wang <ruoyuw560@gmail.com>

component: Release match data on add failure

component_match_add_release() accepts a release callback so callers can
transfer ownership of resources held by the match data. However,
__component_match_add() returns without invoking that callback when the
match is already an error pointer or when allocating the match or its
array fails.

> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 655d68deb590c..2b455d7499926 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -388,14 +388,14 @@ static void __component_match_add(struct device *parent,
>  	struct component_match *match = *matchptr;
>  
>  	if (IS_ERR(match))
> -		return;
> +		goto err_release;
>  
>  	if (!match) {
>  		match = devres_alloc(devm_component_match_release,
>  				     sizeof(*match), GFP_KERNEL);
>  		if (!match) {
>  			*matchptr = ERR_PTR(-ENOMEM);
> -			return;
> +			goto err_release;

[Severity: High]
This is a pre-existing issue, but if __component_match_add() fails here and
sets *matchptr to ERR_PTR(-ENOMEM), could this result in a kernel panic
later?

Callers such as drm_of_component_probe() in drivers/gpu/drm/drm_of.c often
pass the resulting match pointer directly to
component_master_add_with_match() without checking IS_ERR(match):

drivers/gpu/drm/drm_of.c:drm_of_component_probe() {
    ...
    return component_master_add_with_match(dev, m_ops, match);
}

component_master_add_with_match() then appears to blindly dereference the
pointer:

drivers/base/component.c:component_master_add_with_match() {
    ...
    ret = component_match_realloc(match, match->num);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814134132.1387952-1-ruoyuw560@gmail.com?part=1