[PATCH] coresight: fix resource leaks on path build failure
Jie Gan
jie.gan at oss.qualcomm.com
Tue May 19 18:55:56 PDT 2026
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, module,
>> 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 at oss.qualcomm.com>
>> ---
>> 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.
>
> 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
>
> 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 at linaro.org>
>
More information about the linux-arm-kernel
mailing list