Re: [PATCH RESEND v13 01/14] coresight: etm4x: read-back TRCSEQSTR at disabling and prohibit modifying seq_state while enabling.

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] coresight: TOCTOU race in seq_state_store bypasses active-session guard
--

commit 750a64ecfab03057a253043e06b3def402bb7137
Author: Yeoreum Yun <yeoreum.yun@xxxxxxx>

coresight: etm4x: read-back TRCSEQSTR at disabling and prohibit modifying seq_state while enabling.

TRCSEQSTR can be changed by the ETM while a session is active. Its value
therefore needs to be preserved when a sysfs session is disabled so that
users can read the current sequence state after the session ends.

> 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
> @@ -1435,6 +1435,8 @@ static ssize_t seq_state_store(struct device *dev,
>  
>  	if (kstrtoul(buf, 16, &val))
>  		return -EINVAL;
> +	if (IS_ERR_OR_NULL(drvdata->csdev) || coresight_get_mode(drvdata->csdev))
> +		return -EBUSY;
>  	if (val >= drvdata->nrseqstate)
>  		return -EINVAL;
>  
>  	config->seq_state = val;
        ^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Could this active-session guard be bypassed due to a Time-of-Check to
Time-of-Use race?

If a user-space write to seq_state and an enable_source call occur
concurrently, is it possible for enable_source to succeed immediately
after this mode check passes?

If coresight_take_mode() succeeds in the racing enable_source call, it
would take drvdata->spinlock to copy the configuration for the active
session. This seq_state_store() write would then proceed without a lock,
modifying the configuration after it was already checked.

When the sysfs session is later disabled, wouldn't the old state read
from hardware overwrite the user's new value?

Should raw_spin_lock(&drvdata->spinlock) be taken around this mode check
and the configuration assignment?

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