Re: [PATCH] coresight: fix resource leaks on path build failure
From: Jie Gan
Date: Wed May 20 2026 - 06:19:23 EST
On 5/20/2026 5:27 PM, Mike Leach wrote:
-----Original Message-----
From: James Clark <james.clark@xxxxxxxxxx>
Sent: Wednesday, May 20, 2026 9:38 AM
To: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
Cc: coresight@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx; Suzuki Poulose <Suzuki.Poulose@xxxxxxx>; Mike
Leach <Mike.Leach@xxxxxxx>; Leo Yan <Leo.Yan@xxxxxxx>; Alexander
Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>; Mathieu Poirier
<mathieu.poirier@xxxxxxxxxx>; Tingwei Zhang
<tingwei.zhang@xxxxxxxxxxxxxxxx>; Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] coresight: fix resource leaks on path build failure
On 20/05/2026 2:55 am, Jie Gan wrote:
module,
On 5/19/2026 9:57 PM, James Clark wrote:
On 13/05/2026 2:32 am, Jie Gan wrote:
Two related leaks when _coresight_build_path() encounters an error after
coresight_grab_device() has already incremented the pm_runtime,
and device references for a node:
1. In _coresight_build_path(), if kzalloc_obj() for the path node fails
after coresight_grab_device() succeeds, coresight_drop_device() was
never called, permanently leaking all three references.
2. In coresight_build_path(), on failure the partial path was freed with
kfree(path) instead of coresight_release_path(path). kfree() only
frees the coresight_path struct itself; it does not iterate
path_list
to call coresight_drop_device() and kfree() for each coresight_node
already added by deeper recursive calls, leaking both the
pm_runtime,
module, and device references and the node memory for every element
on the partial path.
Fix both by adding coresight_drop_device() in the OOM unwind of
_coresight_build_path(), and replacing kfree(path) with
coresight_release_path(path) in coresight_build_path().
Fixes: 32b0707a4182 ("coresight: Add try_get_module() in
coresight_grab_device()")
Fixes: b3e94405941e ("coresight: associating path with session rather
than tracer")
Signed-off-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
---
drivers/hwtracing/coresight/coresight-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/
hwtracing/coresight/coresight-core.c
index 46f247f73cf6..c1354ea8e11d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -825,8 +825,10 @@ static int _coresight_build_path(struct
coresight_device *csdev,
return ret;
node = kzalloc_obj(struct coresight_node);
- if (!node)
+ if (!node) {
+ coresight_drop_device(csdev);
return -ENOMEM;
+ }
node->csdev = csdev;
list_add(&node->link, &path->path_list);
@@ -851,7 +853,7 @@ struct coresight_path
*coresight_build_path(struct coresight_device *source,
rc = _coresight_build_path(source, source, sink, path);
if (rc) {
- kfree(path);
+ coresight_release_path(path);
return ERR_PTR(rc);
}
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260513-fix-memory-leak-issue-034b4a45265e
Best regards,
Looks good to me, but sashiko is complaining: https://sashiko.dev/#/
patchset/20260513-fix-memory-leak-issue-
v1-1-49822d7bc7d4%40oss.qualcomm.com
I'm trying to understand why it's saying that, but I think the
scenario is that if there are multiple correct paths to a sink, when
one path partially fails and a second path succeeds you could get a
path_list with some garbage entries in it.
I think the coresight_release_path is added to address this situation.
We suffered the path partially failure, and we need release all nodes
already added to the path.
It wouldn't call coresight_release_path() in this case though. If one
path ends up building to success but a parallel path partially failed
then _coresight_build_path() still returns success. During the search it
would have still added the nodes from the partially failed path to the
path_list. This is only an issue if there are multiple correct paths.
The point here is there are multiple routes from the same source device to the same sink device, am right?
I have no experience on this scenario. So with the scenario, the build_path may succeeded in one route and failed in another route, but finally, the _coresight_build_path still returns success, is that correct?
That's kind of a different and existing issue to the one you've fixed,
and assumes that multiple paths to one sink are possible, which I'm
not sure is supported?
Each path is unique. We only deal with the issue path for balancing the
reference count.
Thanks,
Jie
I'm not exactly sure what you mean by unique, but the same source and
sink could potentially be connected through two different sets of links.
Multiple paths between a source and sink are not permitted under the CoreSight spec.
As Mike mentioned, my understanding is that a source device is only allowed to be added to one valid path—this is what I mean by “unique.”
Thanks,
Jie
If such a system was to be built - then a fix would need to be in the declaration of connections - e.g. miss one path out in the device tree for example. Not up to the Coresight drivers to handle out of specification hardware.
Mike
It might be as easy as breaking the loop early for any return value
other than -ENODEV, but I'll leave it to you to decide whether to do
that here or not.
Reviewed-by: James Clark <james.clark@xxxxxxxxxx>