linux-kernel - Re: [PATCH v2 2/3] coresight: catu: Prevent concurrent PERF and SYSFS mode enablement

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250402134716.GK115840@e132581.arm.com>
Date: Wed, 2 Apr 2025 14:47:16 +0100
From: Leo Yan <leo.yan@....com>
To: Yabin Cui <yabinc@...gle.com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
	Mike Leach <mike.leach@...aro.org>,
	James Clark <james.clark@...aro.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] coresight: catu: Prevent concurrent PERF and
 SYSFS mode enablement

On Tue, Apr 01, 2025 at 06:18:31PM -0700, Yabin Cui wrote:
> A CATU device can be enabled for either PERF mode or SYSFS mode, but
> not both simultaneously. Consider the following race condition:
> 
> CPU 1 enables CATU for PERF mode.
> CPU 2 enables CATU for SYSFS mode. It only increases refcnt.
> CPU 2 enables ETR for SYSFS mode.
> CPU 1 fails to enable ETR for PERF mode.
> 
> This results in a CATU being enabled in PERF mode and an ETR enabled
> in SYSFS mode, leading to unknown behavior.
> 
> To prevent this situation, this patch checks enabled mode of a
> CATU device before increasing its refcnt.

Yes.  We have also observed the same issue.  I am currently working on
refactoring the Arm CoreSight locking, my plan is to use coresight_path
to maintain mode.

Given it is uncommon case to use two modes concurrently in Arm
CoreSight, I assume this is not an urgent issue.  Could we defer this
patch?  My understanding is that this issue will be prevented with
checking the path mode, rather than checking modes in seperate modules.

To be clear, I am fine with other two patches in the series.

Thanks,
Leo

> Signed-off-by: Yabin Cui <yabinc@...gle.com>
> Suggested-by: James Clark <james.clark@...aro.org>
> Suggested-by: Leo Yan <leo.yan@....com>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c | 6 +++++-
>  drivers/hwtracing/coresight/coresight-catu.h | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index b1d490dd7249..0caf3a3e75ce 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -466,7 +466,10 @@ static int catu_enable(struct coresight_device *csdev, enum cs_mode mode,
>  		CS_UNLOCK(catu_drvdata->base);
>  		rc = catu_enable_hw(catu_drvdata, mode, data);
>  		CS_LOCK(catu_drvdata->base);
> -	}
> +		if (!rc)
> +			catu_drvdata->mode = mode;
> +	} else if (catu_drvdata->mode != mode)
> +		rc = -EBUSY;
>  	if (!rc)
>  		csdev->refcnt++;
>  	return rc;
> @@ -499,6 +502,7 @@ static int catu_disable(struct coresight_device *csdev, void *__unused)
>  		CS_UNLOCK(catu_drvdata->base);
>  		rc = catu_disable_hw(catu_drvdata);
>  		CS_LOCK(catu_drvdata->base);
> +		catu_drvdata->mode = CS_MODE_DISABLED;
>  	} else {
>  		rc = -EBUSY;
>  	}
> diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h
> index 755776cd19c5..ea406464f0a8 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.h
> +++ b/drivers/hwtracing/coresight/coresight-catu.h
> @@ -66,6 +66,7 @@ struct catu_drvdata {
>  	struct coresight_device *csdev;
>  	int irq;
>  	raw_spinlock_t spinlock;
> +	enum cs_mode mode;
>  };
>  
>  #define CATU_REG32(name, offset)					\
> -- 
> 2.49.0.472.ge94155a9ec-goog
>