2023-03-06 01:25:00

by Dongliang Mu

[permalink] [raw]
Subject: [PATCH] drivers: loongson: fix memory leak in loongson2_guts_probe/remove

drivers/soc/loongson/loongson2_guts.c:150 loongson2_guts_probe()
warn: 'guts->regs' from ioremap() not released on lines: 131,135,139,143.

Fix this by invoking iounmap() in the error handling code and the remove
function.

Note that, this patch is not tested due to the loongson architecture.

Signed-off-by: Dongliang Mu <[email protected]>
---
drivers/soc/loongson/loongson2_guts.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/loongson/loongson2_guts.c b/drivers/soc/loongson/loongson2_guts.c
index bace4bc8e03b..ba330adb555c 100644
--- a/drivers/soc/loongson/loongson2_guts.c
+++ b/drivers/soc/loongson/loongson2_guts.c
@@ -98,6 +98,7 @@ static int loongson2_guts_probe(struct platform_device *pdev)
const struct loongson2_soc_die_attr *soc_die;
const char *machine;
u32 svr;
+ int rc = -ENOMEM;

/* Initialize guts */
guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
@@ -128,19 +129,21 @@ static int loongson2_guts_probe(struct platform_device *pdev)
soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL, "Loongson");
}
if (!soc_dev_attr.family)
- return -ENOMEM;
+ goto iounmap;
soc_dev_attr.soc_id = devm_kasprintf(dev, GFP_KERNEL,
"svr:0x%08x", svr);
if (!soc_dev_attr.soc_id)
- return -ENOMEM;
+ goto iounmap;
soc_dev_attr.revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
(svr >> 4) & 0xf, svr & 0xf);
if (!soc_dev_attr.revision)
- return -ENOMEM;
+ goto iounmap;

soc_dev = soc_device_register(&soc_dev_attr);
- if (IS_ERR(soc_dev))
- return PTR_ERR(soc_dev);
+ if (IS_ERR(soc_dev)) {
+ rc = PTR_ERR(soc_dev);
+ goto iounmap;
+ }

pr_info("Machine: %s\n", soc_dev_attr.machine);
pr_info("SoC family: %s\n", soc_dev_attr.family);
@@ -148,11 +151,16 @@ static int loongson2_guts_probe(struct platform_device *pdev)
soc_dev_attr.soc_id, soc_dev_attr.revision);

return 0;
+
+iounmap:
+ iounmap(guts->regs);
+ return rc;
}

static int loongson2_guts_remove(struct platform_device *dev)
{
soc_device_unregister(soc_dev);
+ iounmap(guts->regs);

return 0;
}
--
2.39.2



2023-03-06 02:27:25

by WANG Xuerui

[permalink] [raw]
Subject: Re: [PATCH] drivers: loongson: fix memory leak in loongson2_guts_probe/remove

Hi,

On 2023/3/6 09:22, Dongliang Mu wrote:
> drivers/soc/loongson/loongson2_guts.c:150 loongson2_guts_probe()
> warn: 'guts->regs' from ioremap() not released on lines: 131,135,139,143.
>
> Fix this by invoking iounmap() in the error handling code and the remove
> function.
>
> Note that, this patch is not tested due to the loongson architecture.

I can't test either, AFAIK no one outside of Loongson have working
LoongArch Loongson-2 hardware running new world system. Yinbo will have
to take care of this.

>
> Signed-off-by: Dongliang Mu <[email protected]> > ---
> drivers/soc/loongson/loongson2_guts.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/loongson/loongson2_guts.c b/drivers/soc/loongson/loongson2_guts.c
> index bace4bc8e03b..ba330adb555c 100644
> --- a/drivers/soc/loongson/loongson2_guts.c
> +++ b/drivers/soc/loongson/loongson2_guts.c
> @@ -98,6 +98,7 @@ static int loongson2_guts_probe(struct platform_device *pdev)
> const struct loongson2_soc_die_attr *soc_die;
> const char *machine;
> u32 svr;
> + int rc = -ENOMEM;
>
> /* Initialize guts */
> guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
> @@ -128,19 +129,21 @@ static int loongson2_guts_probe(struct platform_device *pdev)
> soc_dev_attr.family = devm_kasprintf(dev, GFP_KERNEL, "Loongson");
> }
> if (!soc_dev_attr.family)
> - return -ENOMEM;
> + goto iounmap;
> soc_dev_attr.soc_id = devm_kasprintf(dev, GFP_KERNEL,
> "svr:0x%08x", svr);
> if (!soc_dev_attr.soc_id)
> - return -ENOMEM;
> + goto iounmap;
> soc_dev_attr.revision = devm_kasprintf(dev, GFP_KERNEL, "%d.%d",
> (svr >> 4) & 0xf, svr & 0xf);
> if (!soc_dev_attr.revision)
> - return -ENOMEM;
> + goto iounmap;

Instead of so many goto's, it's probably better to just change the
ioremap to devm_ioremap and let the driver core take care of the rest.
Much more maintainable this way IMO.

--
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/