Re: [PATCH RESEND v13 03/14] coresight: etm3x: prohibit modifying cntr_val and reset while session is enabled

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] coresight: etm3x: TOCTOU race in reset_store() allows trace ID release during active session
--

commit e2e191629dd66da8998d4646f82c1d5c193cb283
Author: Yeoreum Yun <yeoreum.yun@arm.com>

coresight: etm3x: prohibit modifying cntr_val and reset while session is enabled
    
This commit adds checks to prohibit modifying cntr_val and calling reset while
a trace session is enabled by returning -EBUSY.

> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index b3c67e96a82a4..cdb70a4382eb8 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -75,6 +75,10 @@ static ssize_t reset_store(struct device *dev,
>  	if (ret)
>  		return ret;
>  
> +	if (IS_ERR_OR_NULL(drvdata->csdev) ||
> +	    coresight_get_mode(drvdata->csdev))
> +		return -EBUSY;
> +
>  	if (val) {
>  		spin_lock(&drvdata->spinlock);

[Severity: High]
Does this code introduce a Time-of-Check to Time-of-Use (TOCTOU) race?

Since coresight_get_mode() is checked outside drvdata->spinlock, a concurrent
sysfs write to enable_source could start a trace session (allocating a trace
ID and starting the hardware tracer) immediately after the mode check passes.

Once reset_store() acquires the spinlock, it will unconditionally zero the
sysfs configuration and call etm_release_trace_id().

Releasing the trace ID while the hardware tracer is actively emitting packets
returns the ID to the global pool. Another source might subsequently allocate
the same trace ID, leading to collisions at the CoreSight sink and
unrecoverable trace data corruption. This structural race condition appears
to remain unfixed through the end of the patch series.

>  		memset(config, 0, sizeof(struct etm_config));

[ ... ]

> @@ -744,6 +748,9 @@ static ssize_t cntr_val_store(struct device *dev,
>  	ret = kstrtoul(buf, 16, &val);
>  	if (ret)
>  		return ret;
> +	if (IS_ERR_OR_NULL(drvdata->csdev) ||
> +	    coresight_get_mode(drvdata->csdev))
> +		return -EBUSY;
>  
>  	spin_lock(&drvdata->spinlock);

[Severity: High]
Could the same TOCTOU race pattern apply here as well?

If a trace session is concurrently enabled right after the mode check, the
counter value could be modified while the session is active, resulting in a
lost sysfs write.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003-separate_etm_cfg_v2-v13-0-5d8a29aead95@arm.com?part=3