[PATCH v5 0/3] coresight: Fix possible deadlock in coresight_panic_cb

[PATCH v5 0/3] coresight: Fix possible deadlock in coresight_panic_cb

[Sean Anderson]

This time with the notifier in csdev.

Changes in v5:
- Split reordering of coresight_device_release into a separate patch
- Check csdev mode before calling sync()

Changes in v4:
- Fix fwnode leak in coresight_register error path
- Move the panic notifier into csdev and restore the panic sync API

Changes in v3:
- Rewrite patch to remove the panic sync API entirely

Changes in v2:
- Add a comment describing csdev_lock/list
- Consolidate list removal in coresight_device_release

Sean Anderson (3):
  coresight: Fix fwnode leak in coresight_register error path
  coresight: Reorder coresight_device_release to match
    coresight_register
  coresight: Fix possible deadlock in coresight_panic_cb

 drivers/hwtracing/coresight/coresight-core.c | 71 ++++++++------------
 include/linux/coresight.h                    |  2 +
 2 files changed, 30 insertions(+), 43 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty

[PATCH v5 1/3] coresight: Fix fwnode leak in coresight_register error path

[Sean Anderson]

If registering the CPU map fails, we need to put the fwnode. free_percpu
works when called with a NULL pointer, so just use
coresight_device_release.

Fixes: 5ad628a76176 ("coresight: Use per-sink trace ID maps for Perf sessions")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

(no changes since v4)

Changes in v4:
- New

 drivers/hwtracing/coresight/coresight-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fa758cc21827..022c8384b98d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1352,7 +1352,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 		raw_spin_lock_init(&csdev->perf_sink_id_map.lock);
 		csdev->perf_sink_id_map.cpu_map = alloc_percpu(atomic_t);
 		if (!csdev->perf_sink_id_map.cpu_map) {
-			kfree(csdev);
+			coresight_device_release(&csdev->dev);
 			ret = -ENOMEM;
 			goto err_out;
 		}
-- 
2.35.1.1320.gc452695387.dirty

[PATCH v5 2/3] coresight: Reorder coresight_device_release to match coresight_register

[Sean Anderson]

To make it easier to determine where to add new release actions, reorder
the actions in coresight_device_release to be the reverse of
coresight_register.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v5:
- New

 drivers/hwtracing/coresight/coresight-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 022c8384b98d..305b1773cfbe 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1046,8 +1046,8 @@ static void coresight_device_release(struct device *dev)
 {
 	struct coresight_device *csdev = to_coresight_device(dev);
 
-	fwnode_handle_put(csdev->dev.fwnode);
 	free_percpu(csdev->perf_sink_id_map.cpu_map);
+	fwnode_handle_put(csdev->dev.fwnode);
 	kfree(csdev);
 }
 
-- 
2.35.1.1320.gc452695387.dirty

[PATCH v5 3/3] coresight: Fix possible deadlock in coresight_panic_cb

[Sean Anderson]

Panics can occur at any time, so taking locks may cause a deadlock (such
as if the panicking CPU held the lock).  coresight_panic_cb uses
bus_for_each_dev, but that calls bus_to_subsys which takes
bus_kset->list_lock.

Instead of registering a single panic notifier and iterating over
coresight devices, register a panic notifier for each coresight device
that requires it (letting the atomic notifier list handle iteration).
atomic_notifier_chain_unregister will just return -ENOENT if a notifier
block isn't on the list, so it's safe to call when we haven't registered
a notifier.

Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v5:
- Check csdev mode before calling sync()

Changes in v4:
- Move the panic notifier into csdev and restore the panic sync API

Changes in v3:
- Rewrite patch to remove the panic sync API entirely

Changes in v2:
- Add a comment describing csdev_lock/list
- Consolidate list removal in coresight_device_release

 drivers/hwtracing/coresight/coresight-core.c | 67 ++++++++------------
 include/linux/coresight.h                    |  2 +
 2 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 305b1773cfbe..01303348fc0e 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1046,6 +1046,9 @@ static void coresight_device_release(struct device *dev)
 {
 	struct coresight_device *csdev = to_coresight_device(dev);
 
+	if (panic_ops(csdev))
+		atomic_notifier_chain_unregister(&panic_notifier_list,
+						 &csdev->panic_notifier);
 	free_percpu(csdev->perf_sink_id_map.cpu_map);
 	fwnode_handle_put(csdev->dev.fwnode);
 	kfree(csdev);
@@ -1315,6 +1318,18 @@ void coresight_release_platform_data(struct coresight_device *csdev,
 		coresight_remove_conns_sysfs_group(csdev);
 }
 
+static int coresight_panic_notifier(struct notifier_block *nb,
+				    unsigned long action, void *data)
+{
+	struct coresight_device *csdev =
+		container_of(nb, struct coresight_device, panic_notifier);
+	int mode = coresight_get_mode(csdev);
+
+	if (mode == CS_MODE_SYSFS || mode == CS_MODE_PERF)
+		panic_ops(csdev)->sync(csdev);
+	return NOTIFY_DONE;
+}
+
 struct coresight_device *coresight_register(struct coresight_desc *desc)
 {
 	int ret;
@@ -1357,6 +1372,17 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 			goto err_out;
 		}
 	}
+
+	if (panic_ops(csdev)) {
+		csdev->panic_notifier.notifier_call = coresight_panic_notifier;
+		ret = atomic_notifier_chain_register(&panic_notifier_list,
+						     &csdev->panic_notifier);
+		if (ret) {
+			coresight_device_release(&csdev->dev);
+			goto err_out;
+		}
+	}
+
 	/*
 	 * Make sure the device registration and the connection fixup
 	 * are synchronised, so that we don't see uninitialised devices
@@ -1563,36 +1589,6 @@ const struct bus_type coresight_bustype = {
 	.name	= "coresight",
 };
 
-static int coresight_panic_sync(struct device *dev, void *data)
-{
-	int mode;
-	struct coresight_device *csdev;
-
-	/* Run through panic sync handlers for all enabled devices */
-	csdev = container_of(dev, struct coresight_device, dev);
-	mode = coresight_get_mode(csdev);
-
-	if ((mode == CS_MODE_SYSFS) || (mode == CS_MODE_PERF)) {
-		if (panic_ops(csdev))
-			panic_ops(csdev)->sync(csdev);
-	}
-
-	return 0;
-}
-
-static int coresight_panic_cb(struct notifier_block *self,
-			       unsigned long v, void *p)
-{
-	bus_for_each_dev(&coresight_bustype, NULL, NULL,
-				 coresight_panic_sync);
-
-	return 0;
-}
-
-static struct notifier_block coresight_notifier = {
-	.notifier_call = coresight_panic_cb,
-};
-
 static int __init coresight_init(void)
 {
 	int ret;
@@ -1605,20 +1601,11 @@ static int __init coresight_init(void)
 	if (ret)
 		goto exit_bus_unregister;
 
-	/* Register function to be called for panic */
-	ret = atomic_notifier_chain_register(&panic_notifier_list,
-					     &coresight_notifier);
-	if (ret)
-		goto exit_perf;
-
 	/* initialise the coresight syscfg API */
 	ret = cscfg_init();
 	if (!ret)
 		return 0;
 
-	atomic_notifier_chain_unregister(&panic_notifier_list,
-					     &coresight_notifier);
-exit_perf:
 	etm_perf_exit();
 exit_bus_unregister:
 	bus_unregister(&coresight_bustype);
@@ -1628,8 +1615,6 @@ static int __init coresight_init(void)
 static void __exit coresight_exit(void)
 {
 	cscfg_exit();
-	atomic_notifier_chain_unregister(&panic_notifier_list,
-					     &coresight_notifier);
 	etm_perf_exit();
 	bus_unregister(&coresight_bustype);
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4ac65c68bbf4..a7aaf9d3d01d 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -280,6 +280,7 @@ struct coresight_trace_id_map {
  * @config_csdev_list:  List of system configurations added to the device.
  * @cscfg_csdev_lock:	Protect the lists of configurations and features.
  * @active_cscfg_ctxt:  Context information for current active system configuration.
+ * @panic_notifier: Notifier block used to clean up during a panic
  */
 struct coresight_device {
 	struct coresight_platform_data *pdata;
@@ -304,6 +305,7 @@ struct coresight_device {
 	struct list_head config_csdev_list;
 	raw_spinlock_t cscfg_csdev_lock;
 	void *active_cscfg_ctxt;
+	struct notifier_block panic_notifier;
 };
 
 /*
-- 
2.35.1.1320.gc452695387.dirty

Re: [PATCH v5 3/3] coresight: Fix possible deadlock in coresight_panic_cb

[Leo Yan]

On Thu, Sep 25, 2025 at 11:03:42AM -0400, Sean Anderson wrote:
> Panics can occur at any time, so taking locks may cause a deadlock (such
> as if the panicking CPU held the lock).  coresight_panic_cb uses
> bus_for_each_dev, but that calls bus_to_subsys which takes
> bus_kset->list_lock.
> 
> Instead of registering a single panic notifier and iterating over
> coresight devices, register a panic notifier for each coresight device
> that requires it (letting the atomic notifier list handle iteration).
> atomic_notifier_chain_unregister will just return -ENOENT if a notifier
> block isn't on the list, so it's safe to call when we haven't registered
> a notifier.
> 
> Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks")
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>

Reviewed-by: Leo Yan <leo.yan@arm.com>

Re: [PATCH v5 1/3] coresight: Fix fwnode leak in coresight_register error path

[Mike Leach]

On Thu, 25 Sept 2025 at 16:04, Sean Anderson <sean.anderson@linux.dev> wrote:
>
> If registering the CPU map fails, we need to put the fwnode. free_percpu
> works when called with a NULL pointer, so just use
> coresight_device_release.
>
> Fixes: 5ad628a76176 ("coresight: Use per-sink trace ID maps for Perf sessions")
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - New
>
>  drivers/hwtracing/coresight/coresight-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index fa758cc21827..022c8384b98d 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1352,7 +1352,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>                 raw_spin_lock_init(&csdev->perf_sink_id_map.lock);
>                 csdev->perf_sink_id_map.cpu_map = alloc_percpu(atomic_t);
>                 if (!csdev->perf_sink_id_map.cpu_map) {
> -                       kfree(csdev);
> +                       coresight_device_release(&csdev->dev);
>                         ret = -ENOMEM;
>                         goto err_out;
>                 }
> --
> 2.35.1.1320.gc452695387.dirty
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v5 2/3] coresight: Reorder coresight_device_release to match coresight_register

[Mike Leach]

On Thu, 25 Sept 2025 at 16:04, Sean Anderson <sean.anderson@linux.dev> wrote:
>
> To make it easier to determine where to add new release actions, reorder
> the actions in coresight_device_release to be the reverse of
> coresight_register.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
> Changes in v5:
> - New
>
>  drivers/hwtracing/coresight/coresight-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index 022c8384b98d..305b1773cfbe 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1046,8 +1046,8 @@ static void coresight_device_release(struct device *dev)
>  {
>         struct coresight_device *csdev = to_coresight_device(dev);
>
> -       fwnode_handle_put(csdev->dev.fwnode);
>         free_percpu(csdev->perf_sink_id_map.cpu_map);
> +       fwnode_handle_put(csdev->dev.fwnode);
>         kfree(csdev);
>  }
>
> --
> 2.35.1.1320.gc452695387.dirty
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK