Re: [PATCH 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 01:33:45AM +0000, sashiko-bot@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] null-deref: NULL pointer dereference in seq_state_store() during probe
> - [Medium] race: TOCTOU race in seq_state_store() due to missing spinlock
> --
> 
> commit be18b7cdd3b23e4d67d68cc4591ca50a52fb8ee1
> Author: Yeoreum Yun <yeoreum.yun@xxxxxxx>
> 
> coresight: etm4x: read-back TRCSEQSTR at disabling and prohibit modifying seq_state while enabling.
> 
> This commit copies the TRCSEQSTR value when disabling a sysfs session. It also
> prohibits modifying seq_state while the sysfs session is active to prevent
> those changes from being lost when the session is disabled.
> 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> > index cc6cdd3ae29d5..2c72ea831fe65 100644
> > --- 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) || coresight_get_mode(drvdata->csdev))
> 
> [Severity: High]
> Is there a risk of a NULL pointer dereference on drvdata->csdev during device
> probe?
> 
> coresight_register() exposes sysfs files via device_register() before
> returning the csdev pointer to etm4_probe(). If a user process accesses
> the seq_state sysfs file during this window, drvdata->csdev will still
> be NULL.
> 
> Because IS_ERR_OR_NULL(drvdata) does not check drvdata->csdev, calling
> coresight_get_mode(drvdata->csdev) may dereference a NULL pointer.

Thanks. This is typo and I'll fix it/

> 
> > +		return -EBUSY;
> >  	if (val >= drvdata->nrseqstate)
> >  		return -EINVAL;
> >  
> >  	config->seq_state = val;
> 
> [Severity: Medium]
> Could this assignment result in a TOCTOU race condition?
> 
> The mode check and assignment are performed without taking drvdata->spinlock.
> If a user writes to the seq_state sysfs file while another thread
> simultaneously enables the trace session, the lockless assignment could
> overwrite the state even if the session becomes active.
> 
> Other store functions in this driver correctly use
> raw_spin_lock(&drvdata->spinlock) when modifying configuration state.
> 
> >  	return size;
> >  }

This will be fixed by following patch by separating the curr_config and
active config. so ignorable.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261002-separate_etm_cfg_v2-v13-0-fe429d8d96fb@xxxxxxx?part=1

-- 
Sincerely,
Yeoreum Yun