| 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
| ||
|
Message-ID: <ebbe2e22-e094-4d3a-bac8-b6ed8a8e1ef6@oss.qualcomm.com>
Date: Sat, 20 Sep 2025 09:36:45 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Carl Worth <carl@...amperecomputing.com>,
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>,
Sabrina Dubroca <sd@...asysnail.net>,
Jie Gan <quic_jiegan@...cinc.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] coresight: Fix data arguments to coresight
enable/disable helpers
On 9/20/2025 1:49 AM, Carl Worth wrote:
> In the commit being fixed, coresight_enable_path() was changed to call
> coresight_enable_helpers() by passing a struct coresight_path*
> as the final void* 'path' argument, rather passing 'sink_data'
> as was being passed previously. This set the groundwork for the
> subsequent addition of the ctcu_enable function which interprets
> void* data as a struct coresight_path*, but it also broke the
> existing catu_enable function which interprets void* data
> as a struct perf_output_handle*.
>
> The compiler could not flag the error since there are several layers
> of function calls treating the pointer as void*.
>
> Fix both users simultaneously by adding an additional argument to the
> enable helper interface. So now both the struct coresight_path and the
> struct perf_output_handle pointers are passed. Also, eliminate all
> usages of the void* from these code paths and use explicit types
> instead to make all of this code more robust.
>
> Note: The disable function is also changed from void* to
> struct coresight_path* but no implementations of this function
> need the struct perf_output_handle* so it is not added to the interface.
>
> The existing bug can be reproduced with:
>
> # perf record -e cs_etm//k -C 0-9 dd if=/dev/zero of=/dev/null
>
> Showing an oops as follows:
>
> Unable to handle kernel paging request at virtual address 000f6e84934ed19e
>
> Call trace:
> tmc_etr_get_buffer+0x30/0x80 [coresight_tmc] (P)
> catu_enable_hw+0xbc/0x3d0 [coresight_catu]
> catu_enable+0x70/0xe0 [coresight_catu]
> coresight_enable_path+0xb0/0x258 [coresight]
>
> Fixes: 080ee83cc361 ("Coresight: Change functions to accept the coresight_path")
> Signed-off-by: Carl Worth <carl@...amperecomputing.com>
> ---
>
> Jie, I looked into your suggestion of adding the struct
> perf_output_handle* to the struct coresight_path, but I couldn't
> justify adding event-related data to the path structure, which has
> nothing to do with it.
The AUX event only available in perf mode. Each subsystem can access it
from the path if needed. Since the Coresight system operates by
constructing a path — and this path is passed to each device as data -
it’s acceptable to include all relevant parameters in the
coresight_path, provided the devices along the path require them.
Do you want me to create a patch as example?
Thanks,
Jie
>
> So, instead I took the approach in this patch to plumb both arguments
> through the enable path, (and changed away from void* as you agreed
> to).
>
> Note: I've tested that this fixes the bug for catu_enable. This patch
> also obviously touches ctcu_enable, which I believe is correct, but I
> have not tested since I don't have ready access to the necessary
> hardware. I will appreciate other testing.
>
> -Carl
>
> drivers/hwtracing/coresight/coresight-catu.c | 12 ++++++----
> drivers/hwtracing/coresight/coresight-core.c | 23 +++++++++++--------
> .../hwtracing/coresight/coresight-ctcu-core.c | 11 ++++-----
> .../hwtracing/coresight/coresight-cti-core.c | 7 ++++--
> .../hwtracing/coresight/coresight-cti-sysfs.c | 2 +-
> drivers/hwtracing/coresight/coresight-cti.h | 6 +++--
> drivers/hwtracing/coresight/coresight-priv.h | 2 +-
> .../hwtracing/coresight/coresight-tmc-etr.c | 4 ++--
> drivers/hwtracing/coresight/coresight-tmc.h | 3 ++-
> include/linux/coresight.h | 6 +++--
> 10 files changed, 45 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index 5058432233da..724c25d0afa4 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.c
> +++ b/drivers/hwtracing/coresight/coresight-catu.c
> @@ -397,7 +397,7 @@ static int catu_wait_for_ready(struct catu_drvdata *drvdata)
> }
>
> static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode,
> - void *data)
> + struct perf_output_handle *handle)
> {
> int rc;
> u32 control, mode;
> @@ -425,7 +425,7 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode,
> etrdev = coresight_find_input_type(
> csdev->pdata, CORESIGHT_DEV_TYPE_SINK, etr_subtype);
> if (etrdev) {
> - etr_buf = tmc_etr_get_buffer(etrdev, cs_mode, data);
> + etr_buf = tmc_etr_get_buffer(etrdev, cs_mode, handle);
> if (IS_ERR(etr_buf))
> return PTR_ERR(etr_buf);
> }
> @@ -455,7 +455,8 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode,
> }
>
> static int catu_enable(struct coresight_device *csdev, enum cs_mode mode,
> - void *data)
> + struct coresight_path *path,
> + struct perf_output_handle *handle)
> {
> int rc = 0;
> struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);
> @@ -463,7 +464,7 @@ static int catu_enable(struct coresight_device *csdev, enum cs_mode mode,
> guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock);
> if (csdev->refcnt == 0) {
> CS_UNLOCK(catu_drvdata->base);
> - rc = catu_enable_hw(catu_drvdata, mode, data);
> + rc = catu_enable_hw(catu_drvdata, mode, handle);
> CS_LOCK(catu_drvdata->base);
> }
> if (!rc)
> @@ -488,7 +489,8 @@ static int catu_disable_hw(struct catu_drvdata *drvdata)
> return rc;
> }
>
> -static int catu_disable(struct coresight_device *csdev, void *__unused)
> +static int catu_disable(struct coresight_device *csdev,
> + struct coresight_path *__unused)
> {
> int rc = 0;
> struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index fa758cc21827..cc449d5196a4 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -353,14 +353,17 @@ static bool coresight_is_helper(struct coresight_device *csdev)
> }
>
> static int coresight_enable_helper(struct coresight_device *csdev,
> - enum cs_mode mode, void *data)
> + enum cs_mode mode,
> + struct coresight_path *path,
> + struct perf_output_handle *handle)
> {
> - return helper_ops(csdev)->enable(csdev, mode, data);
> + return helper_ops(csdev)->enable(csdev, mode, path, handle);
> }
>
> -static void coresight_disable_helper(struct coresight_device *csdev, void *data)
> +static void coresight_disable_helper(struct coresight_device *csdev,
> + struct coresight_path *path)
> {
> - helper_ops(csdev)->disable(csdev, data);
> + helper_ops(csdev)->disable(csdev, path);
> }
>
> static void coresight_disable_helpers(struct coresight_device *csdev, void *data)
> @@ -477,7 +480,9 @@ void coresight_disable_path(struct coresight_path *path)
> EXPORT_SYMBOL_GPL(coresight_disable_path);
>
> static int coresight_enable_helpers(struct coresight_device *csdev,
> - enum cs_mode mode, void *data)
> + enum cs_mode mode,
> + struct coresight_path *path,
> + struct perf_output_handle *handle)
> {
> int i, ret = 0;
> struct coresight_device *helper;
> @@ -487,7 +492,7 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
> if (!helper || !coresight_is_helper(helper))
> continue;
>
> - ret = coresight_enable_helper(helper, mode, data);
> + ret = coresight_enable_helper(helper, mode, path, handle);
> if (ret)
> return ret;
> }
> @@ -496,7 +501,7 @@ static int coresight_enable_helpers(struct coresight_device *csdev,
> }
>
> int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> - void *sink_data)
> + struct perf_output_handle *handle)
> {
> int ret = 0;
> u32 type;
> @@ -510,7 +515,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> type = csdev->type;
>
> /* Enable all helpers adjacent to the path first */
> - ret = coresight_enable_helpers(csdev, mode, path);
> + ret = coresight_enable_helpers(csdev, mode, path, handle);
> if (ret)
> goto err_disable_path;
> /*
> @@ -526,7 +531,7 @@ int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
>
> switch (type) {
> case CORESIGHT_DEV_TYPE_SINK:
> - ret = coresight_enable_sink(csdev, mode, sink_data);
> + ret = coresight_enable_sink(csdev, mode, handle);
> /*
> * Sink is the first component turned on. If we
> * failed to enable the sink, there are no components
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index c6bafc96db96..9f6d71c59d4e 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> @@ -156,17 +156,16 @@ static int ctcu_set_etr_traceid(struct coresight_device *csdev, struct coresight
> return __ctcu_set_etr_traceid(csdev, traceid, port_num, enable);
> }
>
> -static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, void *data)
> +static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode,
> + struct coresight_path *path,
> + struct perf_output_handle *handle)
> {
> - struct coresight_path *path = (struct coresight_path *)data;
> -
> return ctcu_set_etr_traceid(csdev, path, true);
> }
>
> -static int ctcu_disable(struct coresight_device *csdev, void *data)
> +static int ctcu_disable(struct coresight_device *csdev,
> + struct coresight_path *path)
> {
> - struct coresight_path *path = (struct coresight_path *)data;
> -
> return ctcu_set_etr_traceid(csdev, path, false);
> }
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 8fb30dd73fd2..f92e3be4607c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -799,14 +799,17 @@ static void cti_pm_release(struct cti_drvdata *drvdata)
> }
>
> /** cti ect operations **/
> -int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data)
> +int cti_enable(struct coresight_device *csdev, enum cs_mode mode,
> + struct coresight_path *path,
> + struct perf_output_handle *handle)
> {
> struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
>
> return cti_enable_hw(drvdata);
> }
>
> -int cti_disable(struct coresight_device *csdev, void *data)
> +int cti_disable(struct coresight_device *csdev,
> + struct coresight_path *path)
> {
> struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index 572b80ee96fb..2bb6929eeb19 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -112,7 +112,7 @@ static ssize_t enable_store(struct device *dev,
> ret = pm_runtime_resume_and_get(dev->parent);
> if (ret)
> return ret;
> - ret = cti_enable(drvdata->csdev, CS_MODE_SYSFS, NULL);
> + ret = cti_enable(drvdata->csdev, CS_MODE_SYSFS, NULL, NULL);
> if (ret)
> pm_runtime_put(dev->parent);
> } else {
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
> index 8362a47c939c..73954369654c 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -216,8 +216,10 @@ int cti_add_connection_entry(struct device *dev, struct cti_drvdata *drvdata,
> const char *assoc_dev_name);
> struct cti_trig_con *cti_allocate_trig_con(struct device *dev, int in_sigs,
> int out_sigs);
> -int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data);
> -int cti_disable(struct coresight_device *csdev, void *data);
> +int cti_enable(struct coresight_device *csdev, enum cs_mode mode,
> + struct coresight_path *path,
> + struct perf_output_handle *handle);
> +int cti_disable(struct coresight_device *csdev, struct coresight_path *path);
> void cti_write_all_hw_regs(struct cti_drvdata *drvdata);
> void cti_write_intack(struct device *dev, u32 ackval);
> void cti_write_single_reg(struct cti_drvdata *drvdata, int offset, u32 value);
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 33e22b1ba043..65c4984d98c0 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -136,7 +136,7 @@ static inline void CS_UNLOCK(void __iomem *addr)
>
> void coresight_disable_path(struct coresight_path *path);
> int coresight_enable_path(struct coresight_path *path, enum cs_mode mode,
> - void *sink_data);
> + struct perf_output_handle *handle);
> struct coresight_device *coresight_get_sink(struct coresight_path *path);
> struct coresight_device *coresight_get_sink_by_id(u32 id);
> struct coresight_device *
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index b07fcdb3fe1a..2ed7fa2366ce 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1325,9 +1325,9 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
> }
>
> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
> - enum cs_mode mode, void *data)
> + enum cs_mode mode,
> + struct perf_output_handle *handle)
> {
> - struct perf_output_handle *handle = data;
> struct etr_perf_buffer *etr_perf;
>
> switch (mode) {
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 6541a27a018e..b6e2b00d393a 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -440,7 +440,8 @@ struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);
> void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu);
> void tmc_etr_remove_catu_ops(void);
> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev,
> - enum cs_mode mode, void *data);
> + enum cs_mode mode,
> + struct perf_output_handle *handle);
> extern const struct attribute_group coresight_etr_group;
>
> #endif
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 4ac65c68bbf4..450cccd46f38 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -422,8 +422,10 @@ struct coresight_ops_source {
> */
> struct coresight_ops_helper {
> int (*enable)(struct coresight_device *csdev, enum cs_mode mode,
> - void *data);
> - int (*disable)(struct coresight_device *csdev, void *data);
> + struct coresight_path *path,
> + struct perf_output_handle *data);
> + int (*disable)(struct coresight_device *csdev,
> + struct coresight_path *path);
> };
>
>
>
> base-commit: 46a51f4f5edade43ba66b3c151f0e25ec8b69cb6
Powered by blists - more mailing lists