2021-12-12 12:50:04

by Xiaoke Wang

Subject: [PATCH] powerpc: mpc52xx_gpt: fix a potential memory leak

When some internal memory errors happend in of_iomap(), we should free
gpt to prevent memory leak.

Signed-off-by: xkernel <[email protected]>
---
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index f862b48..c506cfd 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -722,8 +722,10 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev)
gpt->dev = &ofdev->dev;
gpt->ipb_freq = mpc5xxx_get_bus_frequency(ofdev->dev.of_node);
gpt->regs = of_iomap(ofdev->dev.of_node, 0);
- if (!gpt->regs)
+ if (!gpt->regs) {
+ devm_kfree(&ofdev->dev, gpt);
return -ENOMEM;
+ }

dev_set_drvdata(&ofdev->dev, gpt);

--

2021-12-13 02:07:28

by Michael Ellerman

Subject: Re: [PATCH] powerpc: mpc52xx_gpt: fix a potential memory leak

xkernel <[email protected]> writes:
> When some internal memory errors happend in of_iomap(), we should free
> gpt to prevent memory leak.

But it's allocated with devm_kzalloc(), so the devres core is meant to
free it for us isn't it?

cheers

> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> index f862b48..c506cfd 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -722,8 +722,10 @@ static int mpc52xx_gpt_probe(struct platform_device *ofdev)
> gpt->dev = &ofdev->dev;
> gpt->ipb_freq = mpc5xxx_get_bus_frequency(ofdev->dev.of_node);
> gpt->regs = of_iomap(ofdev->dev.of_node, 0);
> - if (!gpt->regs)
> + if (!gpt->regs) {
> + devm_kfree(&ofdev->dev, gpt);
> return -ENOMEM;
> + }
>
> dev_set_drvdata(&ofdev->dev, gpt);
>
> --

2021-12-13 02:28:52

by Xiaoke Wang

Subject: Re: [PATCH] powerpc: mpc52xx_gpt: fix a potential memory leak

Michael Ellerman <[email protected]> wrote:
> > When some internal memory errors happend in of_iomap(), we should free
> > gpt to prevent memory leak.
>
> But it's allocated with devm_kzalloc(), so the devres core is meant to
> free it for us isn't it?

Yes, maybe you are right. I did that as I mentioned when gpt-regs is NULL, it 
will return -ENOMEM, which is the same when gpt is NULL. So I suppose to
free it in time is better:
> gpt = devm_kzalloc(&ofdev->dev, sizeof *gpt, GFP_KERNEL);
> if (!gpt)
> return -ENOMEM;

cheers