[PATCH] coresight: syscfg: fix deadlock on device registration failure
yingchao deng
dengyingchao at kylinsec.com.cn
Tue Sep 8 18:34:07 PDT 2026
On 09/09/2026 00:16, Leo Yan wrote:
> On Tue, Aug 25, 2026 at 09:47:17AM +0800, yingchao wrote:
>
>> 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.
More information about the linux-arm-kernel
mailing list