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

[Yeoreum Yun]

On Sat, Oct 03, 2026 at 03:08:11AM +0000, sashiko-bot@xxxxxxxxxx wrote:
> 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?

No. seq_state_store() does not update any user configuration.
It only clears the STATUS and PENDING bits.

The other bits in TRCSSCSRn are RO or RES fields, so there is no
user-programmed state here that can be lost or overwritten by a
later hardware readback.

Therefore, the race described above does not result in a lost
configuration update.

-- 
Sincerely,
Yeoreum Yun