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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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