Re: [PATCH v13 02/14] coresight: etm4x: prohibit modifying cntr_val 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 cntr_val_store() defeats active session check
--

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

coresight: etm4x: prohibit modifying cntr_val while session is enabled

This commit adds a check to cntr_val_store() to prevent modifying the
counter value while the coresight session is active. This prevents user
configurations from being discarded when the session is disabled.

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
[ ... ]
> @@ -1615,6 +1615,9 @@ static ssize_t cntr_val_store(struct device *dev,
>  		return -EINVAL;
>  	if (val > ETM_CNTR_MAX_VAL)
>  		return -EINVAL;
> +	if (IS_ERR_OR_NULL(drvdata->csdev) ||
> +	    coresight_get_mode(drvdata->csdev))
> +		return -EBUSY;
>  
>  	raw_spin_lock(&drvdata->spinlock);

[Severity: Medium]
Is there a race condition here between the mode check and acquiring the
spinlock?

If a user-space thread writes to the cntr_val sysfs file while another
thread concurrently starts the trace session, cntr_val_store() evaluates
coresight_get_mode() without holding drvdata->spinlock.

If the trace session is enabled immediately after this check, the hardware
is programmed with the old configuration. The sysfs thread then acquires
the lock and writes the new configuration. When the session is later
disabled, the old hardware state overwrites the configuration array
(config->cntr_val), silently losing the user's update.

Could the mode check be performed inside the drvdata->spinlock critical
section to prevent this sequence?

>  	idx = config->cntr_idx;
>  	config->cntr_val[idx] = val;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-separate_etm_cfg_v2-v13-0-fe429d8d96fb@arm.com?part=2