Re: [PATCH v11 2/9] 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:
- [Medium] coresight: TOCTOU race in sysfs configuration store functions
--

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

coresight: etm3x: prohibit modifying cntr_val and reset while session is enabled

This commit attempts to prevent modifications to cntr_val and reset while
a CoreSight trace session is enabled. It adds a check for the current
mode using coresight_get_mode() before proceeding with the sysfs store
operations.

> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -74,6 +74,8 @@ static ssize_t reset_store(struct device *dev,
>  	ret = kstrtoul(buf, 16, &val);
>  	if (ret)
>  		return ret;
> +	if (coresight_get_mode(drvdata->csdev))
> +		return -EBUSY;
>  
>  	if (val) {
>  		spin_lock(&drvdata->spinlock);

[Severity: Medium]
Could this lockless check introduce a time-of-check to time-of-use race?

If a concurrent thread starts a trace session via the enable_source sysfs
file right after coresight_get_mode() returns false, it might win the race
before spin_lock() is acquired here.

This could result in resetting the configuration of an active trace session
and erroneously releasing its trace ID, potentially leading to trace ID
collisions and hardware trace stream corruption.

[ ... ]

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

[Severity: Medium]
Is this susceptible to a similar race condition as reset_store?

Since coresight_get_mode() is checked outside the spinlock, a concurrent
thread could enable the session just before spin_lock() is acquired,
allowing the counter value to be modified while the session is active.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-separate_etm_cfg_v2-v11-0-d2b258d51747@arm.com?part=2