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@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,
-- 
Jie Gan <jie.gan@oss.qualcomm.com>

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@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.

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?

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@linaro.org>

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@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@linaro.org>
> 

On 20/05/2026 2:55 am, Jie Gan wrote:
> 
> 
> 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@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.
> 

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.

>>
>> 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.

>>
>> 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@linaro.org>
>>
>