linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
@ 2023-04-13  3:24 Zhou Shide
  2023-04-13 19:06 ` Stephen Boyd
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Zhou Shide @ 2023-04-13  3:24 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Bai Ping
  Cc: hust-os-kernel-patches, Zhou Shide, linux-clk, linux-arm-kernel,
	linux-kernel

The function imx8mm_clocks_probe() has two main issues:
- The of_iomap() function may cause a memory leak.
- Memory allocated for 'clk_hw_data' may not be freed properly
in some paths.

To fix these issues, this commit replaces the use of of_iomap()
with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
This ensures that all memory is properly managed and automatically
freed when the device is removed.

In addition, when devm_of_iomap() allocates memory with an error,
it will first jump to label "unregister_hws" and
then return PTR_ ERR(base).

Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
---
The issue is discovered by static analysis, and the patch is not tested yet.

 drivers/clk/imx/clk-imx8mm.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index b618892170f2..e6e0bb4805a6 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -303,7 +303,7 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 	void __iomem *base;
 	int ret;
 
-	clk_hw_data = kzalloc(struct_size(clk_hw_data, hws,
+	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws,
 					  IMX8MM_CLK_END), GFP_KERNEL);
 	if (WARN_ON(!clk_hw_data))
 		return -ENOMEM;
@@ -320,10 +320,12 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MM_CLK_EXT4] = imx_get_clk_hw_by_name(np, "clk_ext4");
 
 	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
-	base = of_iomap(np, 0);
+	base = devm_of_iomap(dev, np, 0, NULL);
 	of_node_put(np);
-	if (WARN_ON(!base))
-		return -ENOMEM;
+	if (WARN_ON(IS_ERR(base))) {
+		ret = PTR_ERR(base);
+		goto unregister_hws;
+	}
 
 	hws[IMX8MM_AUDIO_PLL1_REF_SEL] = imx_clk_hw_mux("audio_pll1_ref_sel", base + 0x0, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
 	hws[IMX8MM_AUDIO_PLL2_REF_SEL] = imx_clk_hw_mux("audio_pll2_ref_sel", base + 0x14, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
@@ -399,8 +401,10 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 
 	np = dev->of_node;
 	base = devm_platform_ioremap_resource(pdev, 0);
-	if (WARN_ON(IS_ERR(base)))
-		return PTR_ERR(base);
+	if (WARN_ON(IS_ERR(base))) {
+		ret = PTR_ERR(base);
+		goto unregister_hws;
+	}
 
 	/* Core Slice */
 	hws[IMX8MM_CLK_A53_DIV] = imx8m_clk_hw_composite_core("arm_a53_div", imx8mm_a53_sels, base + 0x8000);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-13  3:24 [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe' Zhou Shide
@ 2023-04-13 19:06 ` Stephen Boyd
  2023-04-14  2:02   ` 周师德
  2023-04-14 16:38   ` Dan Carpenter
  2023-04-25  2:06 ` 周师德
  2023-04-28  7:50 ` Peng Fan
  2 siblings, 2 replies; 9+ messages in thread
From: Stephen Boyd @ 2023-04-13 19:06 UTC (permalink / raw)
  To: Abel Vesa, Bai Ping, Fabio Estevam, Michael Turquette,
	NXP Linux Team, Peng Fan, Pengutronix Kernel Team, Sascha Hauer,
	Shawn Guo, Zhou Shide
  Cc: hust-os-kernel-patches, Zhou Shide, linux-clk, linux-arm-kernel,
	linux-kernel, Hao Luo

Quoting Zhou Shide (2023-04-12 20:24:39)
> The function imx8mm_clocks_probe() has two main issues:
> - The of_iomap() function may cause a memory leak.
> - Memory allocated for 'clk_hw_data' may not be freed properly
> in some paths.
> 
> To fix these issues, this commit replaces the use of of_iomap()
> with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> This ensures that all memory is properly managed and automatically
> freed when the device is removed.
> 
> In addition, when devm_of_iomap() allocates memory with an error,
> it will first jump to label "unregister_hws" and
> then return PTR_ ERR(base).
> 
> Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
> ---
> The issue is discovered by static analysis, and the patch is not tested yet.

And you're not coordinating with each other?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-13 19:06 ` Stephen Boyd
@ 2023-04-14  2:02   ` 周师德
  2023-04-18 19:57     ` Stephen Boyd
  2023-04-14 16:38   ` Dan Carpenter
  1 sibling, 1 reply; 9+ messages in thread
From: 周师德 @ 2023-04-14  2:02 UTC (permalink / raw)
  To: stephen boyd
  Cc: abel vesa, bai ping, fabio estevam, michael turquette,
	nxp linux team, peng fan, pengutronix kernel team, sascha hauer,
	shawn guo, hust-os-kernel-patches, linux-clk, linux-arm-kernel,
	linux-kernel, hao luo




> -----原始邮件-----
> 发件人: "Stephen Boyd" <sboyd@kernel.org>
> 发送时间: 2023-04-14 03:06:59 (星期五)
> 收件人: "Abel Vesa" <abelvesa@kernel.org>, "Bai Ping" <ping.bai@nxp.com>, "Fabio Estevam" <festevam@gmail.com>, "Michael Turquette" <mturquette@baylibre.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Peng Fan" <peng.fan@nxp.com>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Shawn Guo" <shawnguo@kernel.org>, "Zhou Shide" <u201911681@hust.edu.cn>
> 抄送: hust-os-kernel-patches@googlegroups.com, "Zhou Shide" <u201911681@hust.edu.cn>, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, "Hao Luo" <m202171776@hust.edu.cn>
> 主题: Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
> 
> Quoting Zhou Shide (2023-04-12 20:24:39)
> > The function imx8mm_clocks_probe() has two main issues:
> > - The of_iomap() function may cause a memory leak.
> > - Memory allocated for 'clk_hw_data' may not be freed properly
> > in some paths.
> > 
> > To fix these issues, this commit replaces the use of of_iomap()
> > with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> > This ensures that all memory is properly managed and automatically
> > freed when the device is removed.
> > 
> > In addition, when devm_of_iomap() allocates memory with an error,
> > it will first jump to label "unregister_hws" and
> > then return PTR_ ERR(base).
> > 
> > Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> > Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> > Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
> > ---
> > The issue is discovered by static analysis, and the patch is not tested yet.
> 
> And you're not coordinating with each other?
What do you mean by "coordinating with each other"?

regards,
Zhou Shide
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-13 19:06 ` Stephen Boyd
  2023-04-14  2:02   ` 周师德
@ 2023-04-14 16:38   ` Dan Carpenter
  2023-04-15  1:55     ` Dongliang Mu
  1 sibling, 1 reply; 9+ messages in thread
From: Dan Carpenter @ 2023-04-14 16:38 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Abel Vesa, Bai Ping, Fabio Estevam, Michael Turquette,
	NXP Linux Team, Peng Fan, Pengutronix Kernel Team, Sascha Hauer,
	Shawn Guo, Zhou Shide, hust-os-kernel-patches, linux-clk,
	linux-arm-kernel, linux-kernel, Hao Luo

On Thu, Apr 13, 2023 at 12:06:59PM -0700, Stephen Boyd wrote:
> Quoting Zhou Shide (2023-04-12 20:24:39)
> > The function imx8mm_clocks_probe() has two main issues:
> > - The of_iomap() function may cause a memory leak.
> > - Memory allocated for 'clk_hw_data' may not be freed properly
> > in some paths.
> > 
> > To fix these issues, this commit replaces the use of of_iomap()
> > with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> > This ensures that all memory is properly managed and automatically
> > freed when the device is removed.
> > 
> > In addition, when devm_of_iomap() allocates memory with an error,
> > it will first jump to label "unregister_hws" and
> > then return PTR_ ERR(base).
> > 
> > Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> > Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> > Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
> > ---
> > The issue is discovered by static analysis, and the patch is not tested yet.
> 
> And you're not coordinating with each other?
> 

This is a university program.  The patches are reviewed by his professor
and teaching assistants etc.  I've been reviewing some of these patches
as well because of they're using Smatch.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-14 16:38   ` Dan Carpenter
@ 2023-04-15  1:55     ` Dongliang Mu
  0 siblings, 0 replies; 9+ messages in thread
From: Dongliang Mu @ 2023-04-15  1:55 UTC (permalink / raw)
  To: Dan Carpenter, Stephen Boyd
  Cc: Abel Vesa, Bai Ping, Fabio Estevam, Michael Turquette,
	NXP Linux Team, Peng Fan, Pengutronix Kernel Team, Sascha Hauer,
	Shawn Guo, Zhou Shide, hust-os-kernel-patches, linux-clk,
	linux-arm-kernel, linux-kernel, Hao Luo


On 2023/4/15 00:38, Dan Carpenter wrote:
> On Thu, Apr 13, 2023 at 12:06:59PM -0700, Stephen Boyd wrote:
>> Quoting Zhou Shide (2023-04-12 20:24:39)
>>> The function imx8mm_clocks_probe() has two main issues:
>>> - The of_iomap() function may cause a memory leak.
>>> - Memory allocated for 'clk_hw_data' may not be freed properly
>>> in some paths.
>>>
>>> To fix these issues, this commit replaces the use of of_iomap()
>>> with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
>>> This ensures that all memory is properly managed and automatically
>>> freed when the device is removed.
>>>
>>> In addition, when devm_of_iomap() allocates memory with an error,
>>> it will first jump to label "unregister_hws" and
>>> then return PTR_ ERR(base).
>>>
>>> Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
>>> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
>>> Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
>>> ---
>>> The issue is discovered by static analysis, and the patch is not tested yet.
>> And you're not coordinating with each other?
>>
> This is a university program.  The patches are reviewed by his professor
> and teaching assistants etc.  I've been reviewing some of these patches
> as well because of they're using Smatch.

Thanks for your explanation, Dan. We are from Huazhong University of 
Science and Technology.

Some undergraduate and graduatestudents who are interested in Linux 
Kernel are guided by me [1] and Dan to contribute into our kernel community.

We found Smatch is really great in finding kernel issues and these 
issues are suitable for undergraduate students. Therefore, I contacted 
Dan to do a favor for patch interview. And our internal review are 
publicly hosted in a google group [2].

Please let me know if you have any questions.

[1] https://mudongliang.github.io/about/

[2] https://groups.google.com/g/hust-os-kernel-patches

>
> regards,
> dan carpenter
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-14  2:02   ` 周师德
@ 2023-04-18 19:57     ` Stephen Boyd
  2023-04-19  0:55       ` Dongliang Mu
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2023-04-18 19:57 UTC (permalink / raw)
  To: 周师德
  Cc: abel vesa, bai ping, fabio estevam, michael turquette,
	nxp linux team, peng fan, pengutronix kernel team, sascha hauer,
	shawn guo, hust-os-kernel-patches, linux-clk, linux-arm-kernel,
	linux-kernel, hao luo

Quoting 周师德 (2023-04-13 19:02:19)
> 
> 
> 
> > -----原始邮件-----
> > 发件人: "Stephen Boyd" <sboyd@kernel.org>
> > 发送时间: 2023-04-14 03:06:59 (星期五)
> > 收件人: "Abel Vesa" <abelvesa@kernel.org>, "Bai Ping" <ping.bai@nxp.com>, "Fabio Estevam" <festevam@gmail.com>, "Michael Turquette" <mturquette@baylibre.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Peng Fan" <peng.fan@nxp.com>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Shawn Guo" <shawnguo@kernel.org>, "Zhou Shide" <u201911681@hust.edu.cn>
> > 抄送: hust-os-kernel-patches@googlegroups.com, "Zhou Shide" <u201911681@hust.edu.cn>, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, "Hao Luo" <m202171776@hust.edu.cn>
> > 主题: Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
> > 
> > Quoting Zhou Shide (2023-04-12 20:24:39)
> > > The function imx8mm_clocks_probe() has two main issues:
> > > - The of_iomap() function may cause a memory leak.
> > > - Memory allocated for 'clk_hw_data' may not be freed properly
> > > in some paths.
> > > 
> > > To fix these issues, this commit replaces the use of of_iomap()
> > > with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> > > This ensures that all memory is properly managed and automatically
> > > freed when the device is removed.
> > > 
> > > In addition, when devm_of_iomap() allocates memory with an error,
> > > it will first jump to label "unregister_hws" and
> > > then return PTR_ ERR(base).
> > > 
> > > Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> > > Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> > > Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
> > > ---
> > > The issue is discovered by static analysis, and the patch is not tested yet.
> > 
> > And you're not coordinating with each other?
> What do you mean by "coordinating with each other"?
> 

I see two patches to the same driver from the same university on the
list. Preferably you coordinate and decide who will fix what smatch
warnings.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-18 19:57     ` Stephen Boyd
@ 2023-04-19  0:55       ` Dongliang Mu
  0 siblings, 0 replies; 9+ messages in thread
From: Dongliang Mu @ 2023-04-19  0:55 UTC (permalink / raw)
  To: Stephen Boyd, 周师德
  Cc: abel vesa, bai ping, fabio estevam, michael turquette,
	nxp linux team, peng fan, pengutronix kernel team, sascha hauer,
	shawn guo, hust-os-kernel-patches, linux-clk, linux-arm-kernel,
	linux-kernel, hao luo


On 2023/4/19 03:57, Stephen Boyd wrote:
> Quoting 周师德 (2023-04-13 19:02:19)
>>
>>
>>> -----原始邮件-----
>>> 发件人: "Stephen Boyd" <sboyd@kernel.org>
>>> 发送时间: 2023-04-14 03:06:59 (星期五)
>>> 收件人: "Abel Vesa" <abelvesa@kernel.org>, "Bai Ping" <ping.bai@nxp.com>, "Fabio Estevam" <festevam@gmail.com>, "Michael Turquette" <mturquette@baylibre.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Peng Fan" <peng.fan@nxp.com>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Shawn Guo" <shawnguo@kernel.org>, "Zhou Shide" <u201911681@hust.edu.cn>
>>> 抄送: hust-os-kernel-patches@googlegroups.com, "Zhou Shide" <u201911681@hust.edu.cn>, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, "Hao Luo" <m202171776@hust.edu.cn>
>>> 主题: Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
>>>
>>> Quoting Zhou Shide (2023-04-12 20:24:39)
>>>> The function imx8mm_clocks_probe() has two main issues:
>>>> - The of_iomap() function may cause a memory leak.
>>>> - Memory allocated for 'clk_hw_data' may not be freed properly
>>>> in some paths.
>>>>
>>>> To fix these issues, this commit replaces the use of of_iomap()
>>>> with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
>>>> This ensures that all memory is properly managed and automatically
>>>> freed when the device is removed.
>>>>
>>>> In addition, when devm_of_iomap() allocates memory with an error,
>>>> it will first jump to label "unregister_hws" and
>>>> then return PTR_ ERR(base).
>>>>
>>>> Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
>>>> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
>>>> Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
>>>> ---
>>>> The issue is discovered by static analysis, and the patch is not tested yet.
>>> And you're not coordinating with each other?
>> What do you mean by "coordinating with each other"?
>>
> I see two patches to the same driver from the same university on the
> list. Preferably you coordinate and decide who will fix what smatch
> warnings.

Hi Stephen,

As their advisor, I coordinate and assign smatch warnings to each 
student. I double check our assignment table:

drivers/clk/imx/clk-imx8mn.c:612 imx8mn_clocks_probe() warn: 'base' from 
of_iomap() not released on lines: 612.
drivers/clk/imx/clk-imxrt1050.c:154 imxrt1050_clocks_probe() warn: 
'pll_base' from of_iomap() not released on lines: 108,154.
drivers/clk/imx/clk-imx8mm.c:619 imx8mm_clocks_probe() warn: 'base' from 
of_iomap() not released on lines: 403,619.
drivers/clk/imx/clk-imx8mq.c:611 imx8mq_clocks_probe() warn: 'base' from 
of_iomap() not released on lines: 399,611.

There are four similar warnings from your subsystem. If I understand 
correctly, two students are patching issues from different probe 
functions in the different files since we assign all issues in one file 
to one student. Maybe you mix clk-imx8mn (Hao Luo) and clk-imx8mm(Shide 
Zhou). They only differ in one char.

If I miss anything, please let me know. Next time, I will ask one 
student to fix the issues in one subsystem. This can simply the effort 
spent by other student.

Dongliang Mu

>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-13  3:24 [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe' Zhou Shide
  2023-04-13 19:06 ` Stephen Boyd
@ 2023-04-25  2:06 ` 周师德
  2023-04-28  7:50 ` Peng Fan
  2 siblings, 0 replies; 9+ messages in thread
From: 周师德 @ 2023-04-25  2:06 UTC (permalink / raw)
  To: abel vesa, peng fan, michael turquette, stephen boyd, shawn guo,
	sascha hauer, pengutronix kernel team, fabio estevam,
	nxp linux team, bai ping
  Cc: hust-os-kernel-patches, linux-clk, linux-arm-kernel, linux-kernel




> -----原始邮件-----
> 发件人: "Zhou Shide" <u201911681@hust.edu.cn>
> 发送时间: 2023-04-13 11:24:39 (星期四)
> 收件人: "Abel Vesa" <abelvesa@kernel.org>, "Peng Fan" <peng.fan@nxp.com>, "Michael Turquette" <mturquette@baylibre.com>, "Stephen Boyd" <sboyd@kernel.org>, "Shawn Guo" <shawnguo@kernel.org>, "Sascha Hauer" <s.hauer@pengutronix.de>, "Pengutronix Kernel Team" <kernel@pengutronix.de>, "Fabio Estevam" <festevam@gmail.com>, "NXP Linux Team" <linux-imx@nxp.com>, "Bai Ping" <ping.bai@nxp.com>
> 抄送: hust-os-kernel-patches@googlegroups.com, "Zhou Shide" <u201911681@hust.edu.cn>, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
> 主题: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
> 
> The function imx8mm_clocks_probe() has two main issues:
> - The of_iomap() function may cause a memory leak.
> - Memory allocated for 'clk_hw_data' may not be freed properly
> in some paths.
> 
> To fix these issues, this commit replaces the use of of_iomap()
> with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> This ensures that all memory is properly managed and automatically
> freed when the device is removed.
> 
> In addition, when devm_of_iomap() allocates memory with an error,
> it will first jump to label "unregister_hws" and
> then return PTR_ ERR(base).
> 
> Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> Signed-off-by: Zhou Shide <u201911681@hust.edu.cn>
> ---
> The issue is discovered by static analysis, and the patch is not tested yet.
> 
>  drivers/clk/imx/clk-imx8mm.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> index b618892170f2..e6e0bb4805a6 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -303,7 +303,7 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	int ret;
>  
> -	clk_hw_data = kzalloc(struct_size(clk_hw_data, hws,
> +	clk_hw_data = devm_kzalloc(dev, struct_size(clk_hw_data, hws,
>  					  IMX8MM_CLK_END), GFP_KERNEL);
>  	if (WARN_ON(!clk_hw_data))
>  		return -ENOMEM;
> @@ -320,10 +320,12 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
>  	hws[IMX8MM_CLK_EXT4] = imx_get_clk_hw_by_name(np, "clk_ext4");
>  
>  	np = of_find_compatible_node(NULL, NULL, "fsl,imx8mm-anatop");
> -	base = of_iomap(np, 0);
> +	base = devm_of_iomap(dev, np, 0, NULL);
>  	of_node_put(np);
> -	if (WARN_ON(!base))
> -		return -ENOMEM;
> +	if (WARN_ON(IS_ERR(base))) {
> +		ret = PTR_ERR(base);
> +		goto unregister_hws;
> +	}
>  
>  	hws[IMX8MM_AUDIO_PLL1_REF_SEL] = imx_clk_hw_mux("audio_pll1_ref_sel", base + 0x0, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
>  	hws[IMX8MM_AUDIO_PLL2_REF_SEL] = imx_clk_hw_mux("audio_pll2_ref_sel", base + 0x14, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> @@ -399,8 +401,10 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
>  
>  	np = dev->of_node;
>  	base = devm_platform_ioremap_resource(pdev, 0);
> -	if (WARN_ON(IS_ERR(base)))
> -		return PTR_ERR(base);
> +	if (WARN_ON(IS_ERR(base))) {
> +		ret = PTR_ERR(base);
> +		goto unregister_hws;
> +	}
>  
>  	/* Core Slice */
>  	hws[IMX8MM_CLK_A53_DIV] = imx8m_clk_hw_composite_core("arm_a53_div", imx8mm_a53_sels, base + 0x8000);
> -- 
> 2.34.1
ping?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe'
  2023-04-13  3:24 [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe' Zhou Shide
  2023-04-13 19:06 ` Stephen Boyd
  2023-04-25  2:06 ` 周师德
@ 2023-04-28  7:50 ` Peng Fan
  2 siblings, 0 replies; 9+ messages in thread
From: Peng Fan @ 2023-04-28  7:50 UTC (permalink / raw)
  To: Zhou Shide, Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Bai Ping
  Cc: hust-os-kernel-patches, linux-clk, linux-arm-kernel, linux-kernel



On 4/13/2023 11:24 AM, Zhou Shide wrote:
> The function imx8mm_clocks_probe() has two main issues:
> - The of_iomap() function may cause a memory leak.
> - Memory allocated for 'clk_hw_data' may not be freed properly
> in some paths.
> 
> To fix these issues, this commit replaces the use of of_iomap()
> with devm_of_iomap() and replaces kzalloc() with devm_kzalloc().
> This ensures that all memory is properly managed and automatically
> freed when the device is removed.
> 
> In addition, when devm_of_iomap() allocates memory with an error,
> it will first jump to label "unregister_hws" and
> then return PTR_ ERR(base).
> 
> Fixes: 9c71f9ea35d7 ("clk: imx: imx8mm: Switch to clk_hw based API")
> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> Signed-off-by: Zhou Shide<u201911681@hust.edu.cn>


Reviewed-by: Peng Fan <peng.fan@nxp.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-04-28  7:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-13  3:24 [PATCH] clk: imx: clk-imx8mm: fix memory leak issue in 'imx8mm_clocks_probe' Zhou Shide
2023-04-13 19:06 ` Stephen Boyd
2023-04-14  2:02   ` 周师德
2023-04-18 19:57     ` Stephen Boyd
2023-04-19  0:55       ` Dongliang Mu
2023-04-14 16:38   ` Dan Carpenter
2023-04-15  1:55     ` Dongliang Mu
2023-04-25  2:06 ` 周师德
2023-04-28  7:50 ` Peng Fan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).