Re: [PATCH v19 1/7] coresight: core: refactor ctcu_get_active_port and make it generic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Lockless iteration over `pdata->in_conns` array introduces a TOCTOU race condition and potential Use-After-Free.

Pre-existing issues:
- [High] Missing bounds check for Device Tree-provided `port_num` leads to out-of-bounds array access in CTCU trace ID configuration.
- [High] Unsafe direct dereference of sparse array elements in `tpda_get_element_size`.
--

commit f655e6df0885045f179728cb52f455717556ac67
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: core: refactor ctcu_get_active_port and make it generic

This patch removes ctcu_get_active_port from the CTCU module and adds it to
the core framework as coresight_get_in_port. It allows retrieving the active
input port number generically for a given device.

> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6d65c43d574fd..7a7a85acdca0d 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -773,6 +773,33 @@ struct coresight_device *coresight_get_sink(struct coresight_path *path)
[ ... ]
> +int coresight_get_in_port(struct coresight_device *csdev,
> +			  struct coresight_device *remote)
> +{
> +	struct coresight_platform_data *pdata = remote->pdata;
> +	int i;
> +
> +	for (i = 0; i < pdata->nr_inconns; ++i) {
> +		if (!pdata->in_conns[i])
> +			continue;
> +
> +		if (pdata->in_conns[i]->src_dev == csdev)

[Severity: High]
Can this sequence cause a race condition leading to a NULL pointer
dereference?

The array element is read twice. First, to check if it's NULL, and then
again to access src_dev. If a concurrent device unbind operation like
coresight_remove_conns() sets the element to NULL between these reads, the
second access might trigger a panic.

Should we use READ_ONCE() to load the pointer into a local variable first?

> +			return pdata->in_conns[i]->dest_port;
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(coresight_get_in_port);
> +
>  u32 coresight_get_sink_id(struct coresight_device *csdev)
>  {
>  	if (!csdev->ea)
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index 9043cad42f01e..e8720026c9e3f 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> @@ -145,7 +128,7 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight
>  		return -EINVAL;
>  	}
>  
> -	port_num = ctcu_get_active_port(sink, csdev);
> +	port_num = coresight_get_in_port(sink, csdev);
>  	if (port_num < 0)
>  		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does the returned
port_num need to be bounds-checked before being used?

The port_num originates from device tree properties, and if it exceeds
ETR_MAX_NUM (2), it leads to out-of-bounds array accesses in
__ctcu_set_etr_traceid() when indexing into drvdata->atid_offset and
drvdata->traceid_refcnt:

__ctcu_set_etr_traceid()
    ...
    atid_offset = drvdata->atid_offset[port_num];
    ...
    refcnt = drvdata->traceid_refcnt[port_num][traceid];

Would it be safer to validate port_num against the array bounds after
retrieving it?

>  
[ ... ]

[Severity: High]
This is a pre-existing issue not introduced by this patch, but while
looking at the sparse in_conns array usage, I noticed a similar issue in
tpda_get_element_size().

Does this code dereference the sparse array element before checking if it
is valid?

drivers/hwtracing/coresight/coresight-tpda.c:tpda_get_element_size() {
    ...
    for (i = 0; i < csdev->pdata->nr_inconns; i++) {
        in = csdev->pdata->in_conns[i]->src_dev;
        if (!in)
            continue;
    ...
}

If a component is unbound and the connection is removed by
coresight_remove_conns(), csdev->pdata->in_conns[i] could be NULL,
causing a panic before the check is reached.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=1