Small fixes for bugs in the CoreSight drivers that were found by Sashiko. Mostly reference and resource leaks on error paths, plus a deadlock, a missing lock, and a sysfs store that should return -EBUSY while a sysfs session is enabled.
Assisted by Claude Opus 5.5 and Sashiko
Signed-off-by: James Clark james.clark@linaro.org --- Changes in v2: - Cherry pick "coresight: etm-perf: Serialize AUX topology access" into this set so that it now has no dependencies. - Drop "coresight: cpu-debug: Fix PM usage counter leak in debug_enable_func()" which is fixed more generally by https://lore.kernel.org/linux-arm-kernel/20260911-b4-coresight-runtime-pm-fa...
- Link to v1: https://patch.msgid.link/20261027-james-cs-sashiko-easy-fixes-1-v1-0-db43c46...
--- James Clark (9): coresight: etm-perf: Serialize AUX topology access coresight: etm4x: Remove perf symlink if config registration fails coresight: etm4x: Fix truncation of nr_ext_inp coresight: etm-perf: Fix sink device reference leak in etm_setup_aux() coresight: syscfg: Take csdev lock when removing owned features coresight: syscfg: Fix deadlock in cscfg_create_device() error path coresight: Fix resource leaks on coresight_build_path() errors coresight: etm3x: Don't ignore etm_parse_event_config() errors coresight: etm3x: Prohibit modifying seq_curr_state while session is enabled
drivers/hwtracing/coresight/coresight-core.c | 26 +++++++++++++++++----- drivers/hwtracing/coresight/coresight-etm-perf.c | 15 +++++++++---- drivers/hwtracing/coresight/coresight-etm3x-core.c | 6 ++++- .../hwtracing/coresight/coresight-etm3x-sysfs.c | 5 +++++ drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 + drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- drivers/hwtracing/coresight/coresight-priv.h | 1 + drivers/hwtracing/coresight/coresight-syscfg.c | 8 ++++++- drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++-- 9 files changed, 53 insertions(+), 15 deletions(-) --- base-commit: 343cfcbb3ecfdb80aa576a428cfa3b65ad783842 change-id: 20261027-james-cs-sashiko-easy-fixes-1-692d80016a89
Best regards, -- James Clark james.clark@linaro.org
Perf AUX setup walks the CoreSight connections to select a sink, build paths and acquire references to the devices and their helpers. Unlike sysfs tracing, it does not hold coresight_mutex. Driver registration and removal can therefore change or free connections while setup walks them. Adding a CTI helper can also reallocate an existing device's output connection array even when the device is already referenced by a path.
Hold coresight_mutex from sink selection through buffer allocation. The allocation must also be protected because ETR buffers can look up a connected CATU. Release the mutex before calling etm_free_aux() on error.
AUX cleanup walks helper connections again when releasing paths. Add a locking coresight_release_path() wrapper and use coresight_release_path_locked() for sysfs and setup error paths which already hold the mutex. Leave sink buffer freeing and configuration deactivation outside the mutex. AUX cleanup runs in sleepable context following the preceding workqueue removal.
Add lockdep assertions to path construction and release to document the existing sysfs locking contract and enforce it for perf as well. This protects sleepable AUX topology walks; connection reads in atomic path enable/disable callbacks need a separate solution.
Fixes: 0bcbf2e30ff2 ("coresight: etm-perf: new PMU driver for ETM tracers") Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 20 ++++++++++++++++---- drivers/hwtracing/coresight/coresight-etm-perf.c | 11 +++++++---- drivers/hwtracing/coresight/coresight-priv.h | 1 + drivers/hwtracing/coresight/coresight-sysfs.c | 4 ++-- 4 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index d2f9c1fb77d9..c05d657cde73 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -30,8 +30,8 @@ #include "coresight-trace-id.h"
/* - * Mutex used to lock all sysfs enable and disable actions and loading and - * unloading devices by the Coresight core. + * Mutex used to serialize device registration and removal, sysfs enable and + * disable actions, and perf AUX setup and path release. */ DEFINE_MUTEX(coresight_mutex); static DEFINE_PER_CPU(struct coresight_device *, csdev_sink); @@ -910,6 +910,8 @@ static void coresight_drop_device(struct coresight_device *csdev) { int i;
+ lockdep_assert_held(&coresight_mutex); + coresight_put_ref(csdev); for (i = 0; i < csdev->pdata->nr_outconns; i++) { struct coresight_device *child; @@ -1046,6 +1048,8 @@ struct coresight_path *coresight_build_path(struct coresight_device *source, struct coresight_path *path; int rc;
+ lockdep_assert_held(&coresight_mutex); + if (!sink) return ERR_PTR(-EINVAL);
@@ -1065,17 +1069,19 @@ struct coresight_path *coresight_build_path(struct coresight_device *source, }
/** - * coresight_release_path - release a previously built path. + * coresight_release_path_locked - release a path with coresight_mutex held. * @path: the path to release. * * Go through all the elements of a path and 1) removed it from the list and * 2) free the memory allocated for each node. */ -void coresight_release_path(struct coresight_path *path) +void coresight_release_path_locked(struct coresight_path *path) { struct coresight_device *csdev; struct coresight_node *nd, *next;
+ lockdep_assert_held(&coresight_mutex); + list_for_each_entry_safe(nd, next, &path->path_list, link) { csdev = nd->csdev;
@@ -1087,6 +1093,12 @@ void coresight_release_path(struct coresight_path *path) kfree(path); }
+void coresight_release_path(struct coresight_path *path) +{ + guard(mutex)(&coresight_mutex); + coresight_release_path_locked(path); +} + /* return true if the device is a suitable type for a default sink */ static bool coresight_is_def_sink_type(struct coresight_device *csdev) { diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 09b21a711a87..86ec4708b7e4 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -411,7 +411,7 @@ etm_event_build_path(struct perf_event *event, int cpu, /* ensure we can allocate a trace ID for this CPU */ ret = coresight_path_assign_trace_id(path, CS_MODE_PERF); if (ret) { - coresight_release_path(path); + coresight_release_path_locked(path); path = NULL; goto out; } @@ -438,6 +438,9 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, return NULL; INIT_WORK(&event_data->work, free_event_data);
+ /* Protect sink selection, path construction and buffer allocation. */ + mutex_lock(&coresight_mutex); + /* First get the selected sink from user space. */ sink_hash = ATTR_CFG_GET_FLD(&event->attr, sinkid); if (sink_hash) @@ -508,13 +511,13 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, if (!event_data->snk_config) goto err;
-out: + mutex_unlock(&coresight_mutex); return event_data;
err: + mutex_unlock(&coresight_mutex); etm_free_aux(event_data); - event_data = NULL; - goto out; + return NULL; }
static int etm_event_resume(struct coresight_path *path) diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index dddac946659f..edec5a7d2be3 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -143,6 +143,7 @@ coresight_find_default_sink(struct coresight_device *csdev); struct coresight_path *coresight_build_path(struct coresight_device *csdev, struct coresight_device *sink); void coresight_release_path(struct coresight_path *path); +void coresight_release_path_locked(struct coresight_path *path); int coresight_add_sysfs_link(struct coresight_sysfs_link *info); void coresight_remove_sysfs_link(struct coresight_sysfs_link *info); int coresight_create_conns_sysfs_group(struct coresight_device *csdev); diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c index 75000fe64fda..5dd4e9b0426e 100644 --- a/drivers/hwtracing/coresight/coresight-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-sysfs.c @@ -230,7 +230,7 @@ int coresight_enable_sysfs(struct coresight_device *csdev) coresight_disable_path(path);
err_path: - coresight_release_path(path); + coresight_release_path_locked(path); goto out; } EXPORT_SYMBOL_GPL(coresight_enable_sysfs); @@ -256,7 +256,7 @@ void coresight_disable_sysfs(struct coresight_device *csdev) goto out;
coresight_disable_path(path); - coresight_release_path(path); + coresight_release_path_locked(path);
out: mutex_unlock(&coresight_mutex);
If etm4_cscfg_register() fails, the perf symlink created just before it is left behind in the etm_pmu device's directory. Remove it in the same way as etm4_remove_dev() does.
Fixes: 810ac401db1f ("coresight: etm4x: Add complex configuration handlers to etmv4") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-5a385158-37fb-48f1-a8c7-28e86e07848a Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x-core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index e21be6468e95..ba21fe12870b 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2258,6 +2258,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) /* register with config infrastructure & load any current features */ ret = etm4_cscfg_register(drvdata->csdev); if (ret) { + etm_perf_symlink(drvdata->csdev, false); coresight_unregister(drvdata->csdev); return ret; }
TRCIDR5.NUMEXTIN is a 9 bit field but it's stored in a u8, so values of 256 or more are truncated. Make nr_ext_inp a u16.
Fixes: 2e1cdfe184b5 ("coresight-etm4x: Adding CoreSight ETM4x driver") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-b3468d48-ff08-49c9-9ed8-fb6af6979e1e Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 5417280cdb27..8628dd4e15db 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -874,7 +874,7 @@ struct etmv4_caps { u8 nr_pe_cmp; u8 nr_addr_cmp; u8 nr_cntr; - u8 nr_ext_inp; + u16 nr_ext_inp; u8 numcidc; u8 numextinsel; u8 numvmidc;
coresight_get_sink_by_id() returns the sink with a reference taken by bus_find_device(), but etm_setup_aux() never drops it. The paths built for each CPU take their own references on the sink, so drop the lookup reference before returning on both the success and error paths.
Fixes: 226443925887 ("coresight: Use event attributes for sink selection") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-89fe1ee7-7e2f-4c78-a8e6-93a1a260015b Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm-perf.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 86ec4708b7e4..bd302ed1543a 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -511,10 +511,14 @@ static void *etm_setup_aux(struct perf_event *event, void **pages, if (!event_data->snk_config) goto err;
+ if (user_sink) + put_device(&user_sink->dev); mutex_unlock(&coresight_mutex); return event_data;
err: + if (user_sink) + put_device(&user_sink->dev); mutex_unlock(&coresight_mutex); etm_free_aux(event_data); return NULL;
cscfg_remove_owned_csdev_features() modifies csdev->feature_csdev_list without holding csdev->cscfg_csdev_lock, but other users of the list like cscfg_csdev_reset_feats() take it. Take the lock in the same way as cscfg_remove_owned_csdev_configs() already does for the config list.
Fixes: 02bd588e12df ("coresight: configuration: Update API to permit dynamic load/unload") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-6bcce2f2-5aba-40a4-8946-4d843ec84683 Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-syscfg.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 2bfdd7b45e49..8d18cd2aa347 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -410,6 +410,8 @@ static void cscfg_remove_owned_csdev_features(struct coresight_device *csdev, vo if (list_empty(&csdev->feature_csdev_list)) return;
+ guard(raw_spinlock_irqsave)(&csdev->cscfg_csdev_lock); + list_for_each_entry_safe(feat_csdev, tmp, &csdev->feature_csdev_list, node) { if (feat_csdev->feat_desc->load_owner == load_owner) list_del(&feat_csdev->node);
If device_register() fails, put_device() is called with cscfg_mutex held. This calls cscfg_dev_release() which takes cscfg_mutex again and deadlocks. Drop the mutex before calling put_device().
Fixes: 199380decc5f ("coresight: configfs: Fix unload of configurations on module exit") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-e07a8e2f-c34e-4609-a667-9f294c6fcf29 Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-syscfg.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 8d18cd2aa347..1ecf4e9a91c4 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -1212,8 +1212,12 @@ static int cscfg_create_device(void) dev->init_name = "cs_system_cfg";
err = device_register(dev); - if (err) + if (err) { + /* cscfg_dev_release() takes cscfg_mutex */ + mutex_unlock(&cscfg_mutex); put_device(dev); + return err; + }
create_dev_exit_unlock: mutex_unlock(&cscfg_mutex);
If allocating the path node fails in _coresight_build_path(), the references taken by coresight_grab_device() just before are leaked. Drop them before returning.
Also, when _coresight_build_path() fails, nodes for devices further along the path may have already been added and grabbed, but coresight_build_path() only frees the path itself. Use coresight_release_path_locked() to drop and free any nodes too.
Fixes: b3e94405941e ("coresight: associating path with session rather than tracer") Fixes: 32b0707a4182 ("coresight: Add try_get_module() in coresight_grab_device()") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-fe264804-52b6-4b0d-a43e-184e228df19b Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index c05d657cde73..bcfeb1c97127 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1033,8 +1033,10 @@ static int _coresight_build_path(struct coresight_device *csdev, return ret;
node = kzalloc_obj(struct coresight_node); - if (!node) + if (!node) { + coresight_drop_device(csdev); return -ENOMEM; + }
node->csdev = csdev; list_add(&node->link, &path->path_list); @@ -1061,7 +1063,7 @@ struct coresight_path *coresight_build_path(struct coresight_device *source,
rc = _coresight_build_path(source, source, sink, path); if (rc) { - kfree(path); + coresight_release_path_locked(path); return ERR_PTR(rc); }
etm_parse_event_config() can fail, for example if an invalid timestamp level is requested, but etm_enable_perf() ignores the error and enables the tracer anyway. Roll back to disabled mode and return the error instead, like etm4_enable_perf() does.
Fixes: 882d5e112491 ("coresight: etm3x: implementing perf_enable/disable() API") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-bc35b336-f835-40fa-89d9-7a580efc3b53 Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm3x-core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index b670a324bb41..db8413522d72 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -487,7 +487,11 @@ static int etm_enable_perf(struct coresight_device *csdev, return -EBUSY;
/* Configure the tracer based on the session's specifics */ - etm_parse_event_config(drvdata, event); + ret = etm_parse_event_config(drvdata, event); + if (ret) { + coresight_set_mode(csdev, CS_MODE_DISABLED); + return ret; + } drvdata->traceid = path->trace_id;
/* And enable it */
When a sysfs session is disabled, seq_curr_state is read back from the hardware into sysfs_config, overwriting any value written while the session was enabled. Return -EBUSY in that case.
Do the mode check and the write under drvdata->spinlock. etm_enable_sysfs() takes the mode before copying sysfs_config to curr_config under the same lock, so either the new value is included in the copy, or the store sees that the session is enabled. etm_disable_sysfs() does the read back before releasing the mode, so a store can't succeed until it's done.
Fixes: a939fc5a71ad ("coresight-etm: add CoreSight ETM/PTM driver") Reported-by: Sashiko sashiko-bot@kernel.org Closes: https://sashiko.dev/#/bug/linux-d70beb3d-d997-429d-9828-d396e1bbfbb5 Signed-off-by: James Clark james.clark@linaro.org --- drivers/hwtracing/coresight/coresight-etm3x-sysfs.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index d185fb8270e1..08c10fd146d5 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -1014,6 +1014,11 @@ static ssize_t seq_curr_state_store(struct device *dev, if (val > ETM_SEQ_STATE_MAX_VAL) return -EINVAL;
+ guard(spinlock)(&drvdata->spinlock); + if (IS_ERR_OR_NULL(drvdata->csdev) || + coresight_get_mode(drvdata->csdev) == CS_MODE_SYSFS) + return -EBUSY; + sysfs_config->seq_curr_state = val;
return size;
-
James Clark