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

[sashiko-bot@kernel.org]

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@arm.com>

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.

> +		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;
>  }

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