linux-kernel - Re: [PATCH 1/4] uacce: fix for cdev memory leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2b2b530f-6f29-429a-8549-f2fef8d14c24@hisilicon.com>
Date: Thu, 28 Aug 2025 20:59:04 +0800
From: "linwenkai (C)" <linwenkai6@...ilicon.com>
To: Zhangfei Gao <zhangfei.gao@...aro.org>, Chenghai Huang
	<huangchenghai2@...wei.com>
CC: <gregkh@...uxfoundation.org>, <wangzhou1@...ilicon.com>,
	<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
	<linux-crypto@...r.kernel.org>, <fanghao11@...wei.com>,
	<shenyang39@...wei.com>, <qianweili@...wei.com>, <liulongfang@...wei.com>
Subject: Re: [PATCH 1/4] uacce: fix for cdev memory leak


在 2025/8/25 16:20, Zhangfei Gao 写道:
> On Fri, 22 Aug 2025 at 18:39, Chenghai Huang <huangchenghai2@...wei.com> wrote:
>> From: Wenkai Lin <linwenkai6@...ilicon.com>
>>
>> If adding uacce cdev to the system fails, it could be due to two
>> reasons: either the device's devt exists when the failure occurs,
>> or the device_add operation fails. In the latter case, cdev_del
>> will be executed, but in the former case, it will not, leading to a
>> resource leak. Therefore, it is necessary to perform the cdev_del
>> action during abnormal exit.
>>
>> Fixes: 015d239ac014 ("uacce: add uacce driver")
>> Signed-off-by: Wenkai Lin <linwenkai6@...ilicon.com>
>> Signed-off-by: Chenghai Huang <huangchenghai2@...wei.com>
>> ---
>>   drivers/misc/uacce/uacce.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
>> index 42e7d2a2a90c..3604f722ed60 100644
>> --- a/drivers/misc/uacce/uacce.c
>> +++ b/drivers/misc/uacce/uacce.c
>> @@ -519,6 +519,8 @@ EXPORT_SYMBOL_GPL(uacce_alloc);
>>    */
>>   int uacce_register(struct uacce_device *uacce)
>>   {
>> +       int ret;
>> +
>>          if (!uacce)
>>                  return -ENODEV;
>>
>> @@ -529,7 +531,14 @@ int uacce_register(struct uacce_device *uacce)
>>          uacce->cdev->ops = &uacce_fops;
>>          uacce->cdev->owner = THIS_MODULE;
>>
>> -       return cdev_device_add(uacce->cdev, &uacce->dev);
>> +       ret = cdev_device_add(uacce->cdev, &uacce->dev);
>> +       if (ret) {
>> +               cdev_del(uacce->cdev);
> Can cdev_del be called multi-times? since cdev_device_add may call
> cdev_device_add itself.
> Does this introduce another issue?
>
> Not understand the case: "either the device's devt exists when the
> failure occurs,"
> Looks it is cdev_device_add itself issue, or need add check
> if (uacce->dev.devt)
>   cdev_del(uacce->cdev);
> how about other drivers' behavior.

When devt is non-zero, there is still no way to distinguish whether cdev_del

has been executed internally, which might be a  restriction of 
cdev_device_add.

Other drivers often use cdev_init, which does not require consideration

of memory leak, perhaps uacce should use this method too, the plan is to

modify it in v2.

> Thanks