1. Patchew
  2. linux
  3. View series

[PATCH] coresight: Fix possible deadlock in coresight_panic_cb

Sean Anderson posted 1 patch 1 month ago
There is a newer version of this series
drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++----------
include/linux/coresight.h                    |  1 +
2 files changed, 22 insertions(+), 19 deletions(-)
[PATCH] coresight: Fix possible deadlock in coresight_panic_cb
Posted by Sean Anderson 1 month ago 
coresight_panic_cb is called with interrupts disabled during panics.
However, bus_for_each_dev calls bus_to_subsys which takes
bus_kset->list_lock without disabling IRQs. This will cause a deadlock
if a panic occurs while one of the other coresight functions that uses
bus_for_each_dev is running.

Maintain a separate list of coresight devices to access during a panic.

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

 drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++----------
 include/linux/coresight.h                    |  1 +
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fa758cc21827..1f1bf0e2bf92 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev,
 		coresight_remove_conns_sysfs_group(csdev);
 }
 
+static DEFINE_SPINLOCK(csdev_lock);
+static LIST_HEAD(csdev_list);
+
 struct coresight_device *coresight_register(struct coresight_desc *desc)
 {
 	int ret;
@@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
 		goto out_unlock;
 	}
 
+	scoped_guard(spinlock_irq, &csdev_lock)
+		list_add(&csdev->csdev_list, &csdev_list);
+
 	if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
 	    csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
 		ret = etm_perf_add_symlink_sink(csdev);
 
 		if (ret) {
+			scoped_guard(spinlock_irq, &csdev_lock)
+				list_del(&csdev->csdev_list);
 			device_unregister(&csdev->dev);
 			/*
 			 * As with the above, all resources are free'd
@@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev)
 	coresight_remove_conns(csdev);
 	coresight_clear_default_sink(csdev);
 	coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata);
+	scoped_guard(spinlock_irq, &csdev_lock)
+		list_del(&csdev->csdev_list);
 	device_unregister(&csdev->dev);
 }
 EXPORT_SYMBOL_GPL(coresight_unregister);
@@ -1563,28 +1573,20 @@ 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);
+	struct coresight_device *csdev;
+
+	guard(spinlock)(&csdev_lock);
+	list_for_each_entry(csdev, &csdev_list, csdev_list) {
+		/* Run through panic sync handlers for all enabled devices */
+		int mode = coresight_get_mode(csdev);
+
+		if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) &&
+		    panic_ops(csdev))
+			panic_ops(csdev)->sync(csdev);
+	}
 
 	return 0;
 }
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4ac65c68bbf4..a5e62ebd03b5 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -302,6 +302,7 @@ struct coresight_device {
 	/* system configuration and feature lists */
 	struct list_head feature_csdev_list;
 	struct list_head config_csdev_list;
+	struct list_head csdev_list;
 	raw_spinlock_t cscfg_csdev_lock;
 	void *active_cscfg_ctxt;
 };
-- 
2.35.1.1320.gc452695387.dirty
Re: [PATCH] coresight: Fix possible deadlock in coresight_panic_cb
Posted by Suzuki K Poulose 3 weeks, 1 day ago 
Hi Sean

On 28/08/2025 19:17, Sean Anderson wrote:
> coresight_panic_cb is called with interrupts disabled during panics.
> However, bus_for_each_dev calls bus_to_subsys which takes
> bus_kset->list_lock without disabling IRQs. This will cause a deadlock
> if a panic occurs while one of the other coresight functions that uses
> bus_for_each_dev is running.
> 
> Maintain a separate list of coresight devices to access during a panic.

Thanks for the patch. I have a minor comment.

> 
> Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks")
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
>   drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++----------
>   include/linux/coresight.h                    |  1 +
>   2 files changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
> index fa758cc21827..1f1bf0e2bf92 100644
> --- a/drivers/hwtracing/coresight/coresight-core.c
> +++ b/drivers/hwtracing/coresight/coresight-core.c
> @@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev,
>   		coresight_remove_conns_sysfs_group(csdev);
>   }
>   
> +static DEFINE_SPINLOCK(csdev_lock);
> +static LIST_HEAD(csdev_list);

May be add a comment here to mention why we maintain this list ?

> +
>   struct coresight_device *coresight_register(struct coresight_desc *desc)
>   {
>   	int ret;
> @@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>   		goto out_unlock;
>   	}
>   
> +	scoped_guard(spinlock_irq, &csdev_lock)
> +		list_add(&csdev->csdev_list, &csdev_list);
> +
>   	if (csdev->type == CORESIGHT_DEV_TYPE_SINK ||
>   	    csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) {
>   		ret = etm_perf_add_symlink_sink(csdev);
>   
>   		if (ret) {
> +			scoped_guard(spinlock_irq, &csdev_lock)
> +				list_del(&csdev->csdev_list);
Could this be moved to coresight_device_release(), which will be called
when the device gets unregistered ? That way, you don't need it here
and in coresight_unregister()  too.

Rest looks good to me

Suzuki

>   			device_unregister(&csdev->dev);
>   			/*
>   			 * As with the above, all resources are free'd
> @@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev)
>   	coresight_remove_conns(csdev);
>   	coresight_clear_default_sink(csdev);
>   	coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata);
> +	scoped_guard(spinlock_irq, &csdev_lock)
> +		list_del(&csdev->csdev_list);
>   	device_unregister(&csdev->dev);
>   }
>   EXPORT_SYMBOL_GPL(coresight_unregister);
> @@ -1563,28 +1573,20 @@ 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);
> +	struct coresight_device *csdev;
> +
> +	guard(spinlock)(&csdev_lock);
> +	list_for_each_entry(csdev, &csdev_list, csdev_list) {
> +		/* Run through panic sync handlers for all enabled devices */
> +		int mode = coresight_get_mode(csdev);
> +
> +		if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) &&
> +		    panic_ops(csdev))
> +			panic_ops(csdev)->sync(csdev);
> +	}
>   
>   	return 0;
>   }
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 4ac65c68bbf4..a5e62ebd03b5 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -302,6 +302,7 @@ struct coresight_device {
>   	/* system configuration and feature lists */
>   	struct list_head feature_csdev_list;
>   	struct list_head config_csdev_list;
> +	struct list_head csdev_list;
>   	raw_spinlock_t cscfg_csdev_lock;
>   	void *active_cscfg_ctxt;
>   };

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.