Thread (5 messages) 5 messages, 2 authors, 20d ago

Re: [PATCH] coresight: syscfg: fix deadlock on device registration failure

From: yingchao deng <hidden>
Date: 2026-09-09 01:37:07
Also in: lkml

On 09/09/2026 00:16, Leo Yan wrote:
quoted hunk ↗ jump to hunk
On Tue, Aug 25, 2026 at 09:47:17AM +0800, yingchao wrote:
quoted
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index 2bfdd7b45e49..d0e7e4720e46 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -1210,8 +1210,14 @@ static int cscfg_create_device(void)
  	dev->init_name = "cs_system_cfg";
  
  	err = device_register(dev);
-	if (err)
+	if (err) {
+		/* put_device() triggers cscfg_dev_release() which takes
+		 * cscfg_mutex, so drop the lock first to avoid deadlocking.
+		 */
+		mutex_unlock(&cscfg_mutex);
  		put_device(dev);
+		return err;
+	}
As module init and exit are serialized by the kernel, I don't think we
need the mutex to protect the allocation and freeing of cscfg_mgr.

The mutex should only be used for exclusively access cscfg_mgr.
So how about the change below?

---8<---
diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
index 2bfdd7b45e49..2dd0b29f44e4 100644
--- a/drivers/hwtracing/coresight/coresight-syscfg.c
+++ b/drivers/hwtracing/coresight/coresight-syscfg.c
@@ -1173,27 +1173,21 @@ struct device *cscfg_device(void)
  /* Must have a release function or the kernel will complain on module unload */
  static void cscfg_dev_release(struct device *dev)
  {
-	mutex_lock(&cscfg_mutex);
  	kfree(cscfg_mgr);
  	cscfg_mgr = NULL;
-	mutex_unlock(&cscfg_mutex);
  }
  
  /* a device is needed to "own" some kernel elements such as sysfs entries.  */
  static int cscfg_create_device(void)
  {
  	struct device *dev;
-	int err = -ENOMEM;
-
-	mutex_lock(&cscfg_mutex);
-	if (cscfg_mgr) {
-		err = -EINVAL;
-		goto create_dev_exit_unlock;
-	}
+	int err;
  
  	cscfg_mgr = kzalloc_obj(struct cscfg_manager);
  	if (!cscfg_mgr)
-		goto create_dev_exit_unlock;
+		return -ENOMEM;
+
+	mutex_lock(&cscfg_mutex);
  
  	/* initialise the cscfg_mgr structure */
  	INIT_LIST_HEAD(&cscfg_mgr->csdev_desc_list);
@@ -1204,6 +1198,8 @@ static int cscfg_create_device(void)
  	cscfg_mgr->load_state = CSCFG_NONE;
  	raw_spin_lock_init(&cscfg_mgr->sysfs_store_lock);
  
+	mutex_unlock(&cscfg_mutex);
+
  	/* setup the device */
  	dev = cscfg_device();
  	dev->release = cscfg_dev_release;
@@ -1213,8 +1209,6 @@ static int cscfg_create_device(void)
  	if (err)
  		put_device(dev);
  
-create_dev_exit_unlock:
-	mutex_unlock(&cscfg_mutex);
  	return err;
  }
Agreed — module init/exit are serialized, so the mutex around cscfg_mgr 
alloc/free is unnecessary. Removing it from cscfg_dev_release() is the 
cleaner fix and also resolves
the deadlock. I'll send v2 with your suggested change.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help