[PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[James Clark]

of_graph_get_next_endpoint() releases the reference to the previous
endpoint on each iteration, but when parsing fails the loop exits
early meaning the last reference is never dropped.

Fix it by dropping the refcount in the exit condition.

Fixes: d375b356e687 ("coresight: Fix support for sparsely populated ports")
Signed-off-by: James Clark <james.clark@arm.com>
---
 drivers/hwtracing/coresight/coresight-platform.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
index 9d550f5697fa..57a009552cc5 100644
--- a/drivers/hwtracing/coresight/coresight-platform.c
+++ b/drivers/hwtracing/coresight/coresight-platform.c
@@ -297,8 +297,10 @@ static int of_get_coresight_platform_data(struct device *dev,
 			continue;
 
 		ret = of_coresight_parse_endpoint(dev, ep, pdata);
-		if (ret)
+		if (ret) {
+			of_node_put(ep);
 			return ret;
+		}
 	}
 
 	return 0;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[Laurent Pinchart]

Hi James,

Thank you for the patch.

On Wed, May 29, 2024 at 02:36:26PM +0100, James Clark wrote:
> of_graph_get_next_endpoint() releases the reference to the previous
> endpoint on each iteration, but when parsing fails the loop exits
> early meaning the last reference is never dropped.
> 
> Fix it by dropping the refcount in the exit condition.

You can add

Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Fixes: d375b356e687 ("coresight: Fix support for sparsely populated ports")
> Signed-off-by: James Clark <james.clark@arm.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

One a side note, maybe it would be nice to add a new version of the
for_each_endpoint_of_node() macro that would declare the iterator as a
local variable scoped to the loop, making sure the reference always gets
released when we exit the loop.

And now that I've written that, it sounds we could as well have a
version of the macro that doesn't take a new reference to the iterator.
That may be simpler and less error-prone in the end.

> ---
>  drivers/hwtracing/coresight/coresight-platform.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-platform.c b/drivers/hwtracing/coresight/coresight-platform.c
> index 9d550f5697fa..57a009552cc5 100644
> --- a/drivers/hwtracing/coresight/coresight-platform.c
> +++ b/drivers/hwtracing/coresight/coresight-platform.c
> @@ -297,8 +297,10 @@ static int of_get_coresight_platform_data(struct device *dev,
>  			continue;
>  
>  		ret = of_coresight_parse_endpoint(dev, ep, pdata);
> -		if (ret)
> +		if (ret) {
> +			of_node_put(ep);
>  			return ret;
> +		}
>  	}
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[Markus Elfring]

…
> Fix it by dropping the refcount in the exit condition.

                         reference counter?

How do you think about to use the summary phrase
“Fix reference counting leak in of_get_coresight_platform_data()”?

Regards,
Markus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[Greg Kroah-Hartman]

On Wed, May 29, 2024 at 09:30:11PM +0200, Markus Elfring wrote:
> …
> > Fix it by dropping the refcount in the exit condition.
> 
>                          reference counter?
> 
> How do you think about to use the summary phrase
> “Fix reference counting leak in of_get_coresight_platform_data()”?
> 
> Regards,
> Markus

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] coresight: Fix ref leak when of_coresight_parse_endpoint() fails

[Suzuki K Poulose]

On Wed, 29 May 2024 14:36:26 +0100, James Clark wrote:
> of_graph_get_next_endpoint() releases the reference to the previous
> endpoint on each iteration, but when parsing fails the loop exits
> early meaning the last reference is never dropped.
> 
> Fix it by dropping the refcount in the exit condition.
> 
> 
> [...]

Applied, thanks!

[1/1] coresight: Fix ref leak when of_coresight_parse_endpoint() fails
      https://git.kernel.org/coresight/c/7fcb9cb2fe47294e16067c3cfd25332c8662a115

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel