Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Kalle Valo <hidden>
Date: 2019-05-28 12:33:16
Also in:
linux-wireless, lkml
Gen Zhang [off-list ref] writes:
On Tue, May 28, 2019 at 11:39:22AM +0000, Kalle Valo wrote:quoted
Gen Zhang [off-list ref] wrote:quoted
In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(), when this allocation fails, ENOMEM is returned. However, 'pdev_data' and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When platform_device_alloc() returns NULL, we should also free 'pdev_data' and 'glue' before wl1271_probe() ends to prevent leaking memory. Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is error. Further, we should free 'glue->core', 'pdev_data' and 'glue' when this function normally ends to prevent leaking memory. Signed-off-by: Gen Zhang <redacted>Same questions as with similar SDIO patch: https://patchwork.kernel.org/patch/10959049/ Patch set to Changes Requested. -- https://patchwork.kernel.org/patch/10959053/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatchesThanks for your reply, Kalle. I had debate with Jon about this patch. You could kindly refer to lkml: https://lkml.org/lkml/2019/5/23/1547. And I don't think a practical conclusion is made there.
Yeah, I don't see how that thread proves that these patches are correct.
Further, I e-mailed Greg K-H about when should we use devm_kmalloc(). On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:quoted
devm_kmalloc() is used to allocate memory for a driver dev. Comments above the definition and doc (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all imply that allocated the memory is automatically freed on driver attach, no matter allocation fail or not. However, I examined the code, and there are many sites that devm_kfree() is used to free devm_kmalloc(). e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c. So I am totally confused about this issue. Can anybody give me some guidance? When should we use devm_kfree()?He replied: If you "know" you need to free the memory now, call devm_kfree(). If you want to wait for it to be cleaned up latter, like normal, then do not call it. So could please look in to this issue?
Sorry, no time to investigate this in detail. If you think the patches are correct you can resend them and get someone familiar with the driver to provide Reviewed-by, then I will apply them. -- Kalle Valo