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

[Mike Leach]

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