diff mbox series

[net] net: ehea: fix possible memory leak in ehea_register_port()

Message ID 20221022113722.3409846-1-yangyingliang@huawei.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] net: ehea: fix possible memory leak in ehea_register_port() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: edumazet@google.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yang Yingliang Oct. 22, 2022, 11:37 a.m. UTC
dev_set_name() in ehea_register_port() allocates memory for name,
it need be freed when of_device_register() fails, call put_device()
to give up the reference that hold in device_initialize(), so that
it can be freed in kobject_cleanup() when the refcount hit to 0.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Paolo Abeni Oct. 25, 2022, 11:55 a.m. UTC | #1
Hello,

On Sat, 2022-10-22 at 19:37 +0800, Yang Yingliang wrote:
> dev_set_name() in ehea_register_port() allocates memory for name,
> it need be freed when of_device_register() fails, call put_device()
> to give up the reference that hold in device_initialize(), so that
> it can be freed in kobject_cleanup() when the refcount hit to 0.
> 
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/net/ethernet/ibm/ehea/ehea_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> index 294bdbbeacc3..b4aff59b3eb4 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> @@ -2900,6 +2900,7 @@ static struct device *ehea_register_port(struct ehea_port *port,
>  	ret = of_device_register(&port->ofdev);
>  	if (ret) {
>  		pr_err("failed to register device. ret=%d\n", ret);
> +		put_device(&port->ofdev.dev);
>  		goto out;
>  	}

You need to include a suitable Fixes tag into the commit message.
Additionally, if you have a kmemleak splat handy, please include even
that in the commit message.

Thanks!

Paolo
Yang Yingliang Oct. 25, 2022, 12:16 p.m. UTC | #2
Hi,

On 2022/10/25 19:55, Paolo Abeni wrote:
> Hello,
>
> On Sat, 2022-10-22 at 19:37 +0800, Yang Yingliang wrote:
>> dev_set_name() in ehea_register_port() allocates memory for name,
>> it need be freed when of_device_register() fails, call put_device()
>> to give up the reference that hold in device_initialize(), so that
>> it can be freed in kobject_cleanup() when the refcount hit to 0.
>>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/net/ethernet/ibm/ehea/ehea_main.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
>> index 294bdbbeacc3..b4aff59b3eb4 100644
>> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
>> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
>> @@ -2900,6 +2900,7 @@ static struct device *ehea_register_port(struct ehea_port *port,
>>   	ret = of_device_register(&port->ofdev);
>>   	if (ret) {
>>   		pr_err("failed to register device. ret=%d\n", ret);
>> +		put_device(&port->ofdev.dev);
>>   		goto out;
>>   	}
> You need to include a suitable Fixes tag into the commit message.
> Additionally, if you have a kmemleak splat handy, please include even
> that in the commit message.
Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id 
string array")

Afer commit 1fa5ae857bb1 ("driver core: get rid of struct device's
bus_id string array"), the name of device is allocated dynamically.

>
> Thanks!
>
> Paolo
>
>
> .
Yang Yingliang Oct. 25, 2022, 12:50 p.m. UTC | #3
On 2022/10/25 20:16, Yang Yingliang wrote:
> Hi,
>
> On 2022/10/25 19:55, Paolo Abeni wrote:
>> Hello,
>>
>> On Sat, 2022-10-22 at 19:37 +0800, Yang Yingliang wrote:
>>> dev_set_name() in ehea_register_port() allocates memory for name,
>>> it need be freed when of_device_register() fails, call put_device()
>>> to give up the reference that hold in device_initialize(), so that
>>> it can be freed in kobject_cleanup() when the refcount hit to 0.
>>>
>>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>>> ---
>>>   drivers/net/ethernet/ibm/ehea/ehea_main.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c 
>>> b/drivers/net/ethernet/ibm/ehea/ehea_main.c
>>> index 294bdbbeacc3..b4aff59b3eb4 100644
>>> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
>>> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
>>> @@ -2900,6 +2900,7 @@ static struct device 
>>> *ehea_register_port(struct ehea_port *port,
>>>       ret = of_device_register(&port->ofdev);
>>>       if (ret) {
>>>           pr_err("failed to register device. ret=%d\n", ret);
>>> +        put_device(&port->ofdev.dev);
>>>           goto out;
>>>       }
>> You need to include a suitable Fixes tag into the commit message.
>> Additionally, if you have a kmemleak splat handy, please include even
>> that in the commit message.
> Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id 
> string array")
>
> Afer commit 1fa5ae857bb1 ("driver core: get rid of struct device's
> bus_id string array"), the name of device is allocated dynamically.
It should be 1acf2318dd13 ("ehea: dynamic add / remove port"). I will
add it in v2.

Thanks,
Yang
>
>>
>> Thanks!
>>
>> Paolo
>>
>>
>> .
>
> .
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index 294bdbbeacc3..b4aff59b3eb4 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -2900,6 +2900,7 @@  static struct device *ehea_register_port(struct ehea_port *port,
 	ret = of_device_register(&port->ofdev);
 	if (ret) {
 		pr_err("failed to register device. ret=%d\n", ret);
+		put_device(&port->ofdev.dev);
 		goto out;
 	}