| Message ID | 20240529133626.90080-1-james.clark@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Fix ref leak when of_coresight_parse_endpoint() fails | expand |
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;
…
> 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,
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;
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(-)