mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/1] coresight: syscfg: fix deadlock on device registration failure
@ 2026-09-14  7:45 Yingchao Deng
  2026-09-14  7:49 ` [PATCH v2 1/1] " Yingchao Deng
  0 siblings, 1 reply; 4+ messages in thread
From: Yingchao Deng @ 2026-09-14  7:45 UTC (permalink / raw)
  To: Suzuki K Poulose, Alexander Shishkin, Mike Leach, James Clark, Leo Yan
  Cc: coresight, linux-arm-kernel, linux-kernel, dengyingchao, qinyungao

cscfg_create_device() calls put_device() on the error path while holding
cscfg_mutex.  If device_register() fails, put_device() drops the last
reference and invokes cscfg_dev_release(), which takes cscfg_mutex again,
deadlocking.  This removes the unnecessary mutex from cscfg_dev_release().

Changes in v2:
- Per Leo Yan's suggestion, instead of unlocking before put_device(),
  remove the unnecessary mutex from cscfg_dev_release() and take it
  only around cscfg_mgr field initialization.
- Links to v1: https://lore.kernel.org/all/604ED754D4669F40+20260825014717.2346-1-dengyingchao@kylinsec.com.cn/

Yingchao Deng (1):
  coresight: syscfg: fix deadlock on device registration failure

 drivers/hwtracing/coresight/coresight-syscfg.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)
--
2.33.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/1] coresight: syscfg: fix deadlock on device registration failure
  2026-09-14  7:45 [PATCH v2 0/1] coresight: syscfg: fix deadlock on device registration failure Yingchao Deng
@ 2026-09-14  7:49 ` Yingchao Deng
  2026-09-23 10:04   ` Mike Leach
  2026-09-24 16:24   ` Leo Yan
  0 siblings, 2 replies; 4+ messages in thread
From: Yingchao Deng @ 2026-09-14  7:49 UTC (permalink / raw)
  To: Suzuki K Poulose, Alexander Shishkin, Mike Leach, James Clark, Leo Yan
  Cc: coresight, linux-arm-kernel, linux-kernel, dengyingchao, qinyungao

cscfg_create_device() calls put_device() on the error path while holding
cscfg_mutex.  If device_register() fails, put_device() drops the last
reference and invokes cscfg_dev_release(), which takes cscfg_mutex again,
deadlocking.

Module init and exit are serialized by the kernel, so cscfg_mutex is not
needed to protect the allocation and freeing of cscfg_mgr.  Remove the
mutex from cscfg_dev_release() and take it only while cscfg_mgr fields
are being accessed.

Fixes: 199380decc5f ("coresight: configfs: Fix unload of configurations on module exit")
Suggested-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: Yingchao Deng <dengyingchao@kylinsec.com.cn>
---
 drivers/hwtracing/coresight/coresight-syscfg.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

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;
 }
 
-- 
2.33.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] coresight: syscfg: fix deadlock on device registration failure
  2026-09-14  7:49 ` [PATCH v2 1/1] " Yingchao Deng
@ 2026-09-23 10:04   ` Mike Leach
  2026-09-24 16:24   ` Leo Yan
  1 sibling, 0 replies; 4+ messages in thread
From: Mike Leach @ 2026-09-23 10:04 UTC (permalink / raw)
  To: Yingchao Deng, Suzuki K Poulose, Alexander Shishkin, James Clark,
	Leo Yan
  Cc: coresight, linux-arm-kernel, linux-kernel, qinyungao, nd

Reviewed-by: Mike Leach <mike.leach@arm.com>

On 9/14/26 08:49, Yingchao Deng wrote:
> cscfg_create_device() calls put_device() on the error path while holding
> cscfg_mutex.  If device_register() fails, put_device() drops the last
> reference and invokes cscfg_dev_release(), which takes cscfg_mutex again,
> deadlocking.
> 
> Module init and exit are serialized by the kernel, so cscfg_mutex is not
> needed to protect the allocation and freeing of cscfg_mgr.  Remove the
> mutex from cscfg_dev_release() and take it only while cscfg_mgr fields
> are being accessed.
> 
> Fixes: 199380decc5f ("coresight: configfs: Fix unload of configurations on module exit")
> Suggested-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: Yingchao Deng <dengyingchao@kylinsec.com.cn>
> ---
>   drivers/hwtracing/coresight/coresight-syscfg.c | 18 ++++++------------
>   1 file changed, 6 insertions(+), 12 deletions(-)
> 
> 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;
>   }
>   


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] coresight: syscfg: fix deadlock on device registration failure
  2026-09-14  7:49 ` [PATCH v2 1/1] " Yingchao Deng
  2026-09-23 10:04   ` Mike Leach
@ 2026-09-24 16:24   ` Leo Yan
  1 sibling, 0 replies; 4+ messages in thread
From: Leo Yan @ 2026-09-24 16:24 UTC (permalink / raw)
  To: Yingchao Deng
  Cc: Suzuki K Poulose, Alexander Shishkin, Mike Leach, James Clark,
	coresight, linux-arm-kernel, linux-kernel, qinyungao

On Mon, Sep 14, 2026 at 03:49:00PM +0800, Yingchao Deng wrote:
> cscfg_create_device() calls put_device() on the error path while holding
> cscfg_mutex.  If device_register() fails, put_device() drops the last
> reference and invokes cscfg_dev_release(), which takes cscfg_mutex again,
> deadlocking.
> 
> Module init and exit are serialized by the kernel, so cscfg_mutex is not
> needed to protect the allocation and freeing of cscfg_mgr.  Remove the
> mutex from cscfg_dev_release() and take it only while cscfg_mgr fields
> are being accessed.
> 
> Fixes: 199380decc5f ("coresight: configfs: Fix unload of configurations on module exit")
> Suggested-by: Leo Yan <leo.yan@arm.com>
> Signed-off-by: Yingchao Deng <dengyingchao@kylinsec.com.cn>

For this patch:

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

Sashiko reported an issue for null pointer dereference if configfs
init fails fails [1]. It is good to fix it using a separate patch:

@@ -1299,8 +1299,10 @@ int __init cscfg_init(void)

        /* initialise configfs subsystem */
        err = cscfg_configfs_init(cscfg_mgr);
-       if (err)
-               goto exit_err;
+       if (err) {
+               device_unregister(cscfg_device());
+               return err;
+       }

        /* preload built-in configurations */
        err = cscfg_preload(THIS_MODULE);

@Yingchao, do you mind to work out a formal patch for this?

Thanks,
Leo

[1] https://sashiko.dev/#/patchset/6CC680FFAC60931F%2B20260914074900.1711-1-dengyingchao%40kylinsec.com.cn

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-24 16:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14  7:45 [PATCH v2 0/1] coresight: syscfg: fix deadlock on device registration failure Yingchao Deng
2026-09-14  7:49 ` [PATCH v2 1/1] " Yingchao Deng
2026-09-23 10:04   ` Mike Leach
2026-09-24 16:24   ` Leo Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®