UFS drivers that probe defer will endup leaking memory allocated for
clk and regulator names via kstrdup because the structure that is
holding this memory is allocated via devm_* variants which will be
freed during probe defer but the names are never freed.
Use same devm_* variant of kstrdup to free the memory allocated to
name when driver probe defers.
Kmemleak found around 11 leaks on Qualcomm Dragon Board RB5:
unreferenced object 0xffff66f243fb2c00 (size 128):
comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
hex dump (first 32 bytes):
63 6f 72 65 5f 63 6c 6b 00 76 69 72 74 75 61 6c core_clk.virtual
2f 77 6f 72 6b 71 75 65 75 65 2f 73 63 73 69 5f /workqueue/scsi_
backtrace:
[<000000006f788cd1>] slab_post_alloc_hook+0x88/0x410
[<00000000cfd1372b>] __kmalloc_track_caller+0x138/0x230
[<00000000a92ab17b>] kstrdup+0xb0/0x110
[<0000000037263ab6>] ufshcd_pltfrm_init+0x1a8/0x500
[<00000000a20a5caa>] ufs_qcom_probe+0x20/0x58
[<00000000a5e43067>] platform_probe+0x6c/0x118
[<00000000ef686e3f>] really_probe+0xc4/0x330
[<000000005b18792c>] __driver_probe_device+0x88/0x118
[<00000000a5d295e8>] driver_probe_device+0x44/0x158
[<000000007e83f58d>] __device_attach_driver+0xb4/0x128
[<000000004bfa4470>] bus_for_each_drv+0x68/0xd0
[<00000000b89a83bc>] __device_attach+0xec/0x170
[<00000000ada2beea>] device_initial_probe+0x14/0x20
[<0000000079921612>] bus_probe_device+0x9c/0xa8
[<00000000d268bf7c>] deferred_probe_work_func+0x90/0xd0
[<000000009ef64bfa>] process_one_work+0x29c/0x788
unreferenced object 0xffff66f243fb2c80 (size 128):
comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
hex dump (first 32 bytes):
62 75 73 5f 61 67 67 72 5f 63 6c 6b 00 00 00 00 bus_aggr_clk....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
with this patch no memory leaks are reported.
Fixes: aa4976130934 ("ufs: Add regulator enable support")
Fixes: c6e79dacd86f ("ufs: Add clock initialization support")
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/scsi/ufs/ufshcd-pltfrm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 8859c13f4e09..eaeae83b999f 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -91,7 +91,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
clki->min_freq = clkfreq[i];
clki->max_freq = clkfreq[i+1];
- clki->name = kstrdup(name, GFP_KERNEL);
+ clki->name = devm_kstrdup(dev, name, GFP_KERNEL);
if (!strcmp(name, "ref_clk"))
clki->keep_link_active = true;
dev_dbg(dev, "%s: min %u max %u name %s\n", "freq-table-hz",
@@ -126,7 +126,7 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
if (!vreg)
return -ENOMEM;
- vreg->name = kstrdup(name, GFP_KERNEL);
+ vreg->name = devm_kstrdup(dev, name, GFP_KERNEL);
snprintf(prop_name, MAX_PROP_SIZE, "%s-max-microamp", name);
if (of_property_read_u32(np, prop_name, &vreg->max_uA)) {
--
2.21.0
On 14/09/2021 10:22, Srinivas Kandagatla wrote:
> UFS drivers that probe defer will endup leaking memory allocated for
> clk and regulator names via kstrdup because the structure that is
> holding this memory is allocated via devm_* variants which will be
> freed during probe defer but the names are never freed.
>
> Use same devm_* variant of kstrdup to free the memory allocated to
> name when driver probe defers.
>
> Kmemleak found around 11 leaks on Qualcomm Dragon Board RB5:
>
> unreferenced object 0xffff66f243fb2c00 (size 128):
> comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
> hex dump (first 32 bytes):
> 63 6f 72 65 5f 63 6c 6b 00 76 69 72 74 75 61 6c core_clk.virtual
> 2f 77 6f 72 6b 71 75 65 75 65 2f 73 63 73 69 5f /workqueue/scsi_
> backtrace:
> [<000000006f788cd1>] slab_post_alloc_hook+0x88/0x410
> [<00000000cfd1372b>] __kmalloc_track_caller+0x138/0x230
> [<00000000a92ab17b>] kstrdup+0xb0/0x110
> [<0000000037263ab6>] ufshcd_pltfrm_init+0x1a8/0x500
> [<00000000a20a5caa>] ufs_qcom_probe+0x20/0x58
> [<00000000a5e43067>] platform_probe+0x6c/0x118
> [<00000000ef686e3f>] really_probe+0xc4/0x330
> [<000000005b18792c>] __driver_probe_device+0x88/0x118
> [<00000000a5d295e8>] driver_probe_device+0x44/0x158
> [<000000007e83f58d>] __device_attach_driver+0xb4/0x128
> [<000000004bfa4470>] bus_for_each_drv+0x68/0xd0
> [<00000000b89a83bc>] __device_attach+0xec/0x170
> [<00000000ada2beea>] device_initial_probe+0x14/0x20
> [<0000000079921612>] bus_probe_device+0x9c/0xa8
> [<00000000d268bf7c>] deferred_probe_work_func+0x90/0xd0
> [<000000009ef64bfa>] process_one_work+0x29c/0x788
> unreferenced object 0xffff66f243fb2c80 (size 128):
> comm "kworker/u16:0", pid 7, jiffies 4294893319 (age 94.848s)
> hex dump (first 32 bytes):
> 62 75 73 5f 61 67 67 72 5f 63 6c 6b 00 00 00 00 bus_aggr_clk....
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>
> with this patch no memory leaks are reported.
> Fixes: aa4976130934 ("ufs: Add regulator enable support")
> Fixes: c6e79dacd86f ("ufs: Add clock initialization support")
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
Gentle Ping ?
This is not a critical issue, but it will be nice to get this fixed
atleast in 5.16 release.
--srini
> drivers/scsi/ufs/ufshcd-pltfrm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index 8859c13f4e09..eaeae83b999f 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -91,7 +91,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
>
> clki->min_freq = clkfreq[i];
> clki->max_freq = clkfreq[i+1];
> - clki->name = kstrdup(name, GFP_KERNEL);
> + clki->name = devm_kstrdup(dev, name, GFP_KERNEL);
> if (!strcmp(name, "ref_clk"))
> clki->keep_link_active = true;
> dev_dbg(dev, "%s: min %u max %u name %s\n", "freq-table-hz",
> @@ -126,7 +126,7 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
> if (!vreg)
> return -ENOMEM;
>
> - vreg->name = kstrdup(name, GFP_KERNEL);
> + vreg->name = devm_kstrdup(dev, name, GFP_KERNEL);
>
> snprintf(prop_name, MAX_PROP_SIZE, "%s-max-microamp", name);
> if (of_property_read_u32(np, prop_name, &vreg->max_uA)) {
>
On 9/14/21 2:22 AM, Srinivas Kandagatla wrote:
> UFS drivers that probe defer will endup leaking memory allocated for
> clk and regulator names via kstrdup because the structure that is
> holding this memory is allocated via devm_* variants which will be
> freed during probe defer but the names are never freed.
>
> Use same devm_* variant of kstrdup to free the memory allocated to
> name when driver probe defers.
>
> Kmemleak found around 11 leaks on Qualcomm Dragon Board RB5:
Reviewed-by: Bart Van Assche <[email protected]>
Srinivas,
>> UFS drivers that probe defer will endup leaking memory allocated for
>> clk and regulator names via kstrdup because the structure that is
>> holding this memory is allocated via devm_* variants which will be
>> freed during probe defer but the names are never freed.
Applied to 5.16/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On Tue, 14 Sep 2021 10:22:14 +0100, Srinivas Kandagatla wrote:
> UFS drivers that probe defer will endup leaking memory allocated for
> clk and regulator names via kstrdup because the structure that is
> holding this memory is allocated via devm_* variants which will be
> freed during probe defer but the names are never freed.
>
> Use same devm_* variant of kstrdup to free the memory allocated to
> name when driver probe defers.
>
> [...]
Applied to 5.16/scsi-queue, thanks!
[1/1] scsi: ufs: ufshcd-pltfrm: fix memory leak due to probe defer
https://git.kernel.org/mkp/scsi/c/b6ca770ae7f2
--
Martin K. Petersen Oracle Linux Engineering