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 <[email protected]>
---
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
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 <[email protected]>
> Fixes: d375b356e687 ("coresight: Fix support for sparsely populated ports")
> Signed-off-by: James Clark <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
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
…
> 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
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
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 <[email protected]>