Re: [PATCH] coresight: tpda: fix race between refcnt check and register access

[James Clark]

On 08/04/2026 2:23 pm, Jie Gan wrote:
> Several sysfs show/store handlers check csdev->refcnt before acquiring
> the spinlock, then access hardware registers inside the lock.  This is
> a race: between the check and the lock acquisition, the following can
> occur on another CPU:
>
>    CPU 0 (sysfs reader)              CPU 1 (disable path)
>    ─────────────────────────────     ──────────────────────────────────
>    refcnt == 1, check passes
>    [waiting for spinlock]            tpda_disable(): refcnt → 0
>                                      debug clock disabled
>    spinlock acquired
>    readl_relaxed()  ← FAULT/hang
>
> tpda_disable() decrements csdev->refcnt under the same spinlock, so
> moving the refcnt check inside the lock closes the race.
>
> Fix all six affected sysfs handlers: global_flush_req_show/store,
> syncr_mode_show, syncr_count_show, and port_flush_req_show/store.
>
> Fixes: 8e1c358a3b0e ("coresight: tpda: add global_flush_req sysfs node")
> Fixes: 33f04ead7c49 ("coresight: tpda: add logic to configure TPDA_SYNCR register")
> Fixes: a089d585a7f4 ("coresight: tpda: add sysfs node to flush specific port")
> Signed-off-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>

Reviewed-by: James Clark <james.clark@xxxxxxxxxx>

> ---
>   drivers/hwtracing/coresight/coresight-tpda.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 89c8f71f0aff..63a979312463 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -360,10 +360,10 @@ static ssize_t global_flush_req_show(struct device *dev,
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>   	unsigned long val;
>   
> +	guard(spinlock)(&drvdata->spinlock);
>   	if (!drvdata->csdev->refcnt)
>   		return -EINVAL;
>   
> -	guard(spinlock)(&drvdata->spinlock);
>   	val = readl_relaxed(drvdata->base + TPDA_CR);
>   	/* read global_flush_req bit */
>   	val &= TPDA_CR_FLREQ;
> @@ -382,10 +382,13 @@ static ssize_t global_flush_req_store(struct device *dev,
>   	if (kstrtoul(buf, 0, &val))
>   		return -EINVAL;
>   
> -	if (!drvdata->csdev->refcnt || !val)
> +	if (!val)
>   		return -EINVAL;
>   
>   	guard(spinlock)(&drvdata->spinlock);
> +	if (!drvdata->csdev->refcnt)
> +		return -EINVAL;
> +
>   	val = readl_relaxed(drvdata->base + TPDA_CR);
>   	/* set global_flush_req bit */
>   	val |= TPDA_CR_FLREQ;
> @@ -404,10 +407,10 @@ static ssize_t syncr_mode_show(struct device *dev,
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>   	unsigned long val, syncr_val;
>   
> +	guard(spinlock)(&drvdata->spinlock);
>   	if (!drvdata->csdev->refcnt)
>   		return -EINVAL;
>   
> -	guard(spinlock)(&drvdata->spinlock);
>   	syncr_val = readl_relaxed(drvdata->base + TPDA_SYNCR);
>   	val = FIELD_GET(TPDA_SYNCR_MODE_CTRL_MASK, syncr_val);
>   
> @@ -440,10 +443,10 @@ static ssize_t syncr_count_show(struct device *dev,
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>   	unsigned long val;
>   
> +	guard(spinlock)(&drvdata->spinlock);
>   	if (!drvdata->csdev->refcnt)
>   		return -EINVAL;
>   
> -	guard(spinlock)(&drvdata->spinlock);
>   	val = readl_relaxed(drvdata->base + TPDA_SYNCR);
>   	val &= TPDA_SYNCR_COUNT_MASK;
>   
> @@ -478,10 +481,10 @@ static ssize_t port_flush_req_show(struct device *dev,
>   	struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
>   	unsigned long val;
>   
> +	guard(spinlock)(&drvdata->spinlock);
>   	if (!drvdata->csdev->refcnt)
>   		return -EINVAL;
>   
> -	guard(spinlock)(&drvdata->spinlock);
>   	val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
>   
>   	return sysfs_emit(buf, "0x%lx\n", val);
> @@ -498,10 +501,13 @@ static ssize_t port_flush_req_store(struct device *dev,
>   	if (kstrtou32(buf, 0, &val))
>   		return -EINVAL;
>   
> -	if (!drvdata->csdev->refcnt || !val)
> +	if (!val)
>   		return -EINVAL;
>   
>   	guard(spinlock)(&drvdata->spinlock);
> +	if (!drvdata->csdev->refcnt)
> +		return -EINVAL;
> +
>   	CS_UNLOCK(drvdata->base);
>   	writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
>   	CS_LOCK(drvdata->base);
>
> ---
> base-commit: f3e6330d7fe42b204af05a2dbc68b379e0ad179e
> change-id: 20260408-fix-race-condition-issue-d44203254b87
>
> Best regards,