Re: [PATCH v11 09/27] coresight: Grab per-CPU source device during AUX setup

Suzuki K Poulose
2026-05-06T09:09:52Z

On 01/05/2026 17:47, Leo Yan wrote:
> etm_setup_aux() may access a per-CPU source device while ETM module is
> being unloaded, leading to a potential use-after-free.
>
> Therefore, this commit takes references to the device, which is
> sufficient to prevent the csdev from being released. This ensures that
> csdev can be safely accessed.
>
> Refactor the perf path build code into etm_event_build_path(), making
> it easier to use the coresight_{get|put}_percpu_source_ref() pairs.
> Update the comments accordingly to reflect the new flow.
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-core.c | 29 ++++-
> drivers/hwtracing/coresight/coresight-etm-perf.c | 157 +++++++++++++----------
> drivers/hwtracing/coresight/coresight-priv.h | 3 +-
> 3 files changed, 120 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 6da15f2ef9dc9770e7aa79cc94a7ed3d2f3ad871..b01e63fbbb4b990d4f5ec61c8fa4da63dd59a4b9 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -109,13 +109,38 @@ static void coresight_clear_percpu_source(struct coresight_device *csdev)
> per_cpu(csdev_source, csdev->cpu) = NULL;
> }
>
> -struct coresight_device *coresight_get_percpu_source(int cpu)
> +struct coresight_device *coresight_get_percpu_source_ref(int cpu)
> {
> + struct coresight_device *csdev;
> +
> if (WARN_ON(cpu < 0))
> return NULL;
>
> guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> - return per_cpu(csdev_source, cpu);
> +
> + csdev = per_cpu(csdev_source, cpu);
> + if (!csdev)
> + return NULL;
> +
> + /* Make sure csdev is safe to access */

It may be worth adding a comment here :

/*
* Holding a reference to the csdev->dev ensures that the
* coresight_device is live for the caller. The path building
* logic can safely either build a path to the sink or fail
* if the device is being unregistered (if there was a race).
* The caller can skip the "source" device, if no path could
* be built.
*/

Suzuki

> + get_device(&csdev->dev);
> +
> + return csdev;
> +}

> +
> +void coresight_put_percpu_source_ref(struct coresight_device *csdev)
> +{
> + if (!csdev || !coresight_is_percpu_source(csdev))
> + return;
> +
> + guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> +
> + /*
> + * When the device's refcount reaches zero, coresight_device_release()
> + * is invoked. This is safe even in atomic context, as the release
> + * function does not sleep.
> + */
> + put_device(&csdev->dev);
> }
>
> struct coresight_device *coresight_get_source(struct coresight_path *path)
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index d3b13ef130439fd501f88395d0de9dd21b84b827..9950ad481f29eed3bfac8fe4ae3a593b53830617 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -338,6 +338,85 @@ static struct coresight_path *etm_event_get_ctxt_path(struct etm_ctxt *ctxt)
> return path;
> }
>
> +static struct coresight_path *
> +etm_event_build_path(struct perf_event *event, int cpu,
> + struct coresight_device *user_sink,
> + struct coresight_device *match_sink)
> +{
> + struct coresight_path *path = NULL;
> + struct coresight_device *source, *sink;
> +
> + source = coresight_get_percpu_source_ref(cpu);
> +
> + /*
> + * If there is no ETM associated with this CPU or ever we try to trace
> + * on this CPU, we handle it accordingly.
> + */
> + if (!source)
> + return NULL;
> +
> + /*
> + * If AUX pause feature is enabled but the ETM driver does not
> + * support the operations, skip for this source.
> + */
> + if (event->attr.aux_start_paused &&
> + (!source_ops(source)->pause_perf ||
> + !source_ops(source)->resume_perf)) {
> + dev_err_once(&source->dev, "AUX pause is not supported.\n");
> + goto out;
> + }
> +
> + /* If sink has been specified by user, directly use it */
> + if (user_sink) {
> + sink = user_sink;
> + } else {
> + /*
> + * No sink provided - look for a default sink for all the ETMs,
> + * where this event can be scheduled.
> + *
> + * We allocate the sink specific buffers only once for this
> + * event. If the ETMs have different default sink devices, we
> + * can only use a single "type" of sink as the event can carry
> + * only one sink specific buffer. Thus we have to make sure
> + * that the sinks are of the same type and driven by the same
> + * driver, as the one we allocate the buffer for. We don't
> + * trace on a CPU if the sink is not compatible.
> + */
> +
> + /* Find the default sink for this ETM */
> + sink = coresight_find_default_sink(source);
> + if (!sink)
> + goto out;
> +
> + /* Check if this sink compatible with the last sink */
> + if (match_sink && !sinks_compatible(match_sink, sink))
> + goto out;
> + }
> +
> + /*
> + * Building a path doesn't enable it, it simply builds a
> + * list of devices from source to sink that can be
> + * referenced later when the path is actually needed.
> + */
> + path = coresight_build_path(source, sink);
> + if (IS_ERR(path))
> + goto out;
> +
> + /* ensure we can allocate a trace ID for this CPU */
> + coresight_path_assign_trace_id(path, CS_MODE_PERF);
> + if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
> + coresight_release_path(path);
> + path = NULL;
> + goto out;
> + }
> +
> + coresight_trace_id_perf_start(&sink->perf_sink_id_map);
> +
> +out:
> + coresight_put_percpu_source_ref(source);
> + return IS_ERR_OR_NULL(path) ? NULL : path;
> +}
> +
> static void *etm_setup_aux(struct perf_event *event, void **pages,
> int nr_pages, bool overwrite)
> {
> @@ -345,7 +424,7 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> int cpu = event->cpu;
> cpumask_t *mask;
> struct coresight_device *sink = NULL;
> - struct coresight_device *user_sink = NULL, *last_sink = NULL;
> + struct coresight_device *user_sink = NULL;
> struct etm_event_data *event_data = NULL;
>
> event_data = alloc_event_data(cpu);
> @@ -377,80 +456,26 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
> */
> for_each_cpu(cpu, mask) {
> struct coresight_path *path;
> - struct coresight_device *csdev;
>
> - csdev = coresight_get_percpu_source(cpu);
> - /*
> - * If there is no ETM associated with this CPU clear it from
> - * the mask and continue with the rest. If ever we try to trace
> - * on this CPU, we handle it accordingly.
> - */
> - if (!csdev) {
> + path = etm_event_build_path(event, cpu, user_sink, sink);
> + if (!path) {
> + /*
> + * Failed to create a path for the CPU, clear it from
> + * the mask and continue to next one.
> + */
> cpumask_clear_cpu(cpu, mask);
> continue;
> }
>
> - /*
> - * If AUX pause feature is enabled but the ETM driver does not
> - * support the operations, clear this CPU from the mask and
> - * continue to next one.
> - */
> - if (event->attr.aux_start_paused &&
> - (!source_ops(csdev)->pause_perf || !source_ops(csdev)->resume_perf)) {
> - dev_err_once(&csdev->dev, "AUX pause is not supported.\n");
> - cpumask_clear_cpu(cpu, mask);
> - continue;
> - }
>
> /*
> - * No sink provided - look for a default sink for all the ETMs,
> - * where this event can be scheduled.
> - * We allocate the sink specific buffers only once for this
> - * event. If the ETMs have different default sink devices, we
> - * can only use a single "type" of sink as the event can carry
> - * only one sink specific buffer. Thus we have to make sure
> - * that the sinks are of the same type and driven by the same
> - * driver, as the one we allocate the buffer for. As such
> - * we choose the first sink and check if the remaining ETMs
> - * have a compatible default sink. We don't trace on a CPU
> - * if the sink is not compatible.
> - */
> - if (!user_sink) {
> - /* Find the default sink for this ETM */
> - sink = coresight_find_default_sink(csdev);
> - if (!sink) {
> - cpumask_clear_cpu(cpu, mask);
> - continue;
> - }
> -
> - /* Check if this sink compatible with the last sink */
> - if (last_sink && !sinks_compatible(last_sink, sink)) {
> - cpumask_clear_cpu(cpu, mask);
> - continue;
> - }
> - last_sink = sink;
> - }
> -
> - /*
> - * Building a path doesn't enable it, it simply builds a
> - * list of devices from source to sink that can be
> - * referenced later when the path is actually needed.
> + * The first found sink is saved here and passed to
> + * etm_event_build_path() to check whether the remaining ETMs
> + * have a compatible default sink.
> */
> - path = coresight_build_path(csdev, sink);
> - if (IS_ERR(path)) {
> - cpumask_clear_cpu(cpu, mask);
> - continue;
> - }
> -
> - /* ensure we can allocate a trace ID for this CPU */
> - coresight_path_assign_trace_id(path, CS_MODE_PERF);
> - if (!IS_VALID_CS_TRACE_ID(path->trace_id)) {
> - cpumask_clear_cpu(cpu, mask);
> - coresight_release_path(path);
> - continue;
> - }
> + if (!user_sink && !sink)
> + sink = coresight_get_sink(path);
>
> - coresight_trace_id_perf_start(&sink->perf_sink_id_map);
> *etm_event_cpu_path_ptr(event_data, cpu) = path;
> }
>
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 7ce79fa36232bb1b0af768423777bab27cacee95..a1aab67e23db7fdea5139100312b3eb7cd31df51 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -249,7 +249,8 @@ void coresight_add_helper(struct coresight_device *csdev,
> void coresight_set_percpu_sink(int cpu, struct coresight_device *csdev);
> struct coresight_device *coresight_get_percpu_sink(int cpu);
> struct coresight_device *coresight_get_source(struct coresight_path *path);
> -struct coresight_device *coresight_get_percpu_source(int cpu);
> +struct coresight_device *coresight_get_percpu_source_ref(int cpu);
> +void coresight_put_percpu_source_ref(struct coresight_device *csdev);
> void coresight_disable_source(struct coresight_device *csdev, void *data);
> void coresight_pause_source(struct coresight_device *csdev);
> int coresight_resume_source(struct coresight_device *csdev);
>