netdev - Re: [PATCH net] net: hns: fix possible memory leak in hnae_ae

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <bcde51e2-3abf-6ff7-b5a5-c23c7886d2f4@huawei.com>
Date:   Thu, 20 Oct 2022 15:45:17 +0800
From:   Yang Yingliang <yangyingliang@...wei.com>
To:     Leon Romanovsky <leon@...nel.org>
CC:     <netdev@...r.kernel.org>, <yisen.zhuang@...wei.com>,
        <salil.mehta@...wei.com>, <davem@...emloft.net>
Subject: Re: [PATCH net] net: hns: fix possible memory leak in
 hnae_ae_register()


On 2022/10/18 20:58, Leon Romanovsky wrote:
> On Tue, Oct 18, 2022 at 08:24:51PM +0800, Yang Yingliang wrote:
>> Inject fault while probing module, if device_register() fails,
>> but the refcount of kobject is not decreased to 0, the name
>> allocated in dev_set_name() is leaked. Fix this by calling
>> put_device(), so that name can be freed in callback function
>> kobject_cleanup().
>>
>> unreferenced object 0xffff00c01aba2100 (size 128):
>>    comm "systemd-udevd", pid 1259, jiffies 4294903284 (age 294.152s)
>>    hex dump (first 32 bytes):
>>      68 6e 61 65 30 00 00 00 18 21 ba 1a c0 00 ff ff  hnae0....!......
>>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>>    backtrace:
>>      [<0000000034783f26>] slab_post_alloc_hook+0xa0/0x3e0
>>      [<00000000748188f2>] __kmem_cache_alloc_node+0x164/0x2b0
>>      [<00000000ab0743e8>] __kmalloc_node_track_caller+0x6c/0x390
>>      [<000000006c0ffb13>] kvasprintf+0x8c/0x118
>>      [<00000000fa27bfe1>] kvasprintf_const+0x60/0xc8
>>      [<0000000083e10ed7>] kobject_set_name_vargs+0x3c/0xc0
>>      [<000000000b87affc>] dev_set_name+0x7c/0xa0
>>      [<000000003fd8fe26>] hnae_ae_register+0xcc/0x190 [hnae]
>>      [<00000000fe97edc9>] hns_dsaf_ae_init+0x9c/0x108 [hns_dsaf]
>>      [<00000000c36ff1eb>] hns_dsaf_probe+0x548/0x748 [hns_dsaf]
>>
>> Fixes: 6fe6611ff275 ("net: add Hisilicon Network Subsystem hnae framework support")
>> Signed-off-by: Yang Yingliang <yangyingliang@...wei.com>
>> ---
>>   drivers/net/ethernet/hisilicon/hns/hnae.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.c b/drivers/net/ethernet/hisilicon/hns/hnae.c
>> index 00fafc0f8512..430eccea8e5e 100644
>> --- a/drivers/net/ethernet/hisilicon/hns/hnae.c
>> +++ b/drivers/net/ethernet/hisilicon/hns/hnae.c
>> @@ -419,8 +419,10 @@ int hnae_ae_register(struct hnae_ae_dev *hdev, struct module *owner)
>>   	hdev->cls_dev.release = hnae_release;
>>   	(void)dev_set_name(&hdev->cls_dev, "hnae%d", hdev->id);
>                ^^^^^^^^^^^^ this is unexpected in netdev code.
Did you mean the 'void' can be removed ?
>
>>   	ret = device_register(&hdev->cls_dev);
>> -	if (ret)
>> +	if (ret) {
>> +		put_device(&hdev->cls_dev);
>>   		return ret;
>> +	}
>>   
>>   	__module_get(THIS_MODULE);
>          ^^^^^^^^ I'm curious why? I don't see why you need this.
hnae_ae_register() is called from hns_dsaf.ko(hns_dsaf_probe()), the
refcount of module hnae is already be got while loading hns_dsaf.ko
in resolve_symbol(),  so I think this can be removed.

Thanks,
Yang
>
> The change itself is ok.
>
> Thanks,
> Reviewed-by: Leon Romanovsky <leonro@...dia.com>
> .