[PATCH] coresight: fix resource leaks on path build failure

[PATCH] coresight: fix resource leaks on path build failure

[Jie Gan]

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>

Re: [PATCH] coresight: fix resource leaks on path build failure

[James Clark]

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>

Re: [PATCH] coresight: fix resource leaks on path build failure

[Jie Gan]

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

Re: [PATCH] coresight: fix resource leaks on path build failure

[James Clark]

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

RE: [PATCH] coresight: fix resource leaks on path build failure

[Mike Leach]

> -----Original Message-----
> From: James Clark <james.clark@linaro.org>
> Sent: Wednesday, May 20, 2026 9:38 AM
> To: Jie Gan <jie.gan@oss.qualcomm.com>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike
> Leach <Mike.Leach@arm.com>; Leo Yan <Leo.Yan@arm.com>; Alexander
> Shishkin <alexander.shishkin@linux.intel.com>; Mathieu Poirier
> <mathieu.poirier@linaro.org>; Tingwei Zhang
> <tingwei.zhang@oss.qualcomm.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: Re: [PATCH] coresight: fix resource leaks on path build failure
> 
> 
> 
> 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.
> 

Multiple paths between a source and sink are not permitted under the CoreSight spec.

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

Re: [PATCH] coresight: fix resource leaks on path build failure

[Jie Gan]

On 5/20/2026 5:27 PM, Mike Leach wrote:
> 
> 
>> -----Original Message-----
>> From: James Clark <james.clark@linaro.org>
>> Sent: Wednesday, May 20, 2026 9:38 AM
>> To: Jie Gan <jie.gan@oss.qualcomm.com>
>> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; linux-
>> kernel@vger.kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike
>> Leach <Mike.Leach@arm.com>; Leo Yan <Leo.Yan@arm.com>; Alexander
>> Shishkin <alexander.shishkin@linux.intel.com>; Mathieu Poirier
>> <mathieu.poirier@linaro.org>; Tingwei Zhang
>> <tingwei.zhang@oss.qualcomm.com>; Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>
>> Subject: Re: [PATCH] coresight: fix resource leaks on path build failure
>>
>>
>>
>> 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.
>>

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

Re: [PATCH] coresight: fix resource leaks on path build failure

[James Clark]

On 20/05/2026 11:13 am, Jie Gan wrote:
> 
> 
> On 5/20/2026 5:27 PM, Mike Leach wrote:
>>
>>
>>> -----Original Message-----
>>> From: James Clark <james.clark@linaro.org>
>>> Sent: Wednesday, May 20, 2026 9:38 AM
>>> To: Jie Gan <jie.gan@oss.qualcomm.com>
>>> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org; 
>>> linux-
>>> kernel@vger.kernel.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Mike
>>> Leach <Mike.Leach@arm.com>; Leo Yan <Leo.Yan@arm.com>; Alexander
>>> Shishkin <alexander.shishkin@linux.intel.com>; Mathieu Poirier
>>> <mathieu.poirier@linaro.org>; Tingwei Zhang
>>> <tingwei.zhang@oss.qualcomm.com>; Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org>
>>> Subject: Re: [PATCH] coresight: fix resource leaks on path build failure
>>>
>>>
>>>
>>> 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.
>>>
> 
> 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
> 

That's ok then we can ignore this for this patch. But it would be good 
to enforce that in _coresight_build_path() with some kind of assert. Or 
at least add a comment to appease the AI reviewers.

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

Re: [PATCH] coresight: fix resource leaks on path build failure

[Jie Gan]

On 5/13/2026 9: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>

Gentle ping.

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

Re: [PATCH] coresight: fix resource leaks on path build failure

[James Clark]

On 09/11/2026 01:24, Jie Gan wrote:
> 
> 
> On 5/13/2026 9: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>
> 
> Gentle ping.
> 
>> ---
>>   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);

@Suzuki, there's a conflict here. This will need to be 
coresight_release_path_locked() if we also take "coresight: etm-perf: 
Serialize AUX topology access" from my Sashiko fixes.

>>           return ERR_PTR(rc);
>>       }
>>
>> ---
>> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
>> change-id: 20260513-fix-memory-leak-issue-034b4a45265e
>>
>> Best regards,
>