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

[Mike Leach <mike.leach@arm.com>]

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