[PATCH] ARM: mmp: Fix failure to remove sram device

[PATCH] ARM: mmp: Fix failure to remove sram device

[Uwe Kleine-König]

Make sure in .probe() to set driver data before the function is left to
make it possible in .remove() to undo the actions done.

This fixes a potential memory leak and stops returning an error code in
.remove() that is ignored by the driver core anyhow.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 arch/arm/mach-mmp/sram.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c
index 6794e2db1ad5..ecc46c31004f 100644
--- a/arch/arm/mach-mmp/sram.c
+++ b/arch/arm/mach-mmp/sram.c
@@ -72,6 +72,8 @@ static int sram_probe(struct platform_device *pdev)
 	if (!info)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, info);
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
 		dev_err(&pdev->dev, "no memory resource defined\n");
@@ -107,8 +109,6 @@ static int sram_probe(struct platform_device *pdev)
 	list_add(&info->node, &sram_bank_list);
 	mutex_unlock(&sram_lock);
 
-	platform_set_drvdata(pdev, info);
-
 	dev_info(&pdev->dev, "initialized\n");
 	return 0;
 
@@ -127,17 +127,19 @@ static int sram_remove(struct platform_device *pdev)
 	struct sram_bank_info *info;
 
 	info = platform_get_drvdata(pdev);
-	if (info == NULL)
-		return -ENODEV;
 
-	mutex_lock(&sram_lock);
-	list_del(&info->node);
-	mutex_unlock(&sram_lock);
+	if (info->sram_size) {
+		mutex_lock(&sram_lock);
+		list_del(&info->node);
+		mutex_unlock(&sram_lock);
+
+		gen_pool_destroy(info->gpool);
+		iounmap(info->sram_virt);
+		kfree(info->pool_name);
+	}
 
-	gen_pool_destroy(info->gpool);
-	iounmap(info->sram_virt);
-	kfree(info->pool_name);
 	kfree(info);
+
 	return 0;
 }
 
-- 
2.30.2


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

Re: [PATCH] ARM: mmp: Fix failure to remove sram device

[Greg Kroah-Hartman]

On Mon, Jul 26, 2021 at 10:01:58PM +0200, Uwe Kleine-König wrote:
> Make sure in .probe() to set driver data before the function is left to
> make it possible in .remove() to undo the actions done.
> 
> This fixes a potential memory leak and stops returning an error code in
> .remove() that is ignored by the driver core anyhow.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  arch/arm/mach-mmp/sram.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

Re: [PATCH] ARM: mmp: Fix failure to remove sram device

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

[adding soc@kernel.org to Cc]

On Mon, Jul 26, 2021 at 10:01:58PM +0200, Uwe Kleine-König wrote:
> Make sure in .probe() to set driver data before the function is left to
> make it possible in .remove() to undo the actions done.
> 
> This fixes a potential memory leak and stops returning an error code in
> .remove() that is ignored by the driver core anyhow.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This problem is still real and the patch not applied. Who feels
responsible to pick this up?

> ---
>  arch/arm/mach-mmp/sram.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c
> index 6794e2db1ad5..ecc46c31004f 100644
> --- a/arch/arm/mach-mmp/sram.c
> +++ b/arch/arm/mach-mmp/sram.c
> @@ -72,6 +72,8 @@ static int sram_probe(struct platform_device *pdev)
>  	if (!info)
>  		return -ENOMEM;
>  
> +	platform_set_drvdata(pdev, info);
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (res == NULL) {
>  		dev_err(&pdev->dev, "no memory resource defined\n");
> @@ -107,8 +109,6 @@ static int sram_probe(struct platform_device *pdev)
>  	list_add(&info->node, &sram_bank_list);
>  	mutex_unlock(&sram_lock);
>  
> -	platform_set_drvdata(pdev, info);
> -
>  	dev_info(&pdev->dev, "initialized\n");
>  	return 0;
>  
> @@ -127,17 +127,19 @@ static int sram_remove(struct platform_device *pdev)
>  	struct sram_bank_info *info;
>  
>  	info = platform_get_drvdata(pdev);
> -	if (info == NULL)
> -		return -ENODEV;
>  
> -	mutex_lock(&sram_lock);
> -	list_del(&info->node);
> -	mutex_unlock(&sram_lock);
> +	if (info->sram_size) {
> +		mutex_lock(&sram_lock);
> +		list_del(&info->node);
> +		mutex_unlock(&sram_lock);
> +
> +		gen_pool_destroy(info->gpool);
> +		iounmap(info->sram_virt);
> +		kfree(info->pool_name);
> +	}
>  
> -	gen_pool_destroy(info->gpool);
> -	iounmap(info->sram_virt);
> -	kfree(info->pool_name);
>  	kfree(info);
> +
>  	return 0;
>  }

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ARM: mmp: Fix failure to remove sram device

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 2236 bytes --]

[adding soc@kernel.org to Cc]

On Mon, Jul 26, 2021 at 10:01:58PM +0200, Uwe Kleine-König wrote:
> Make sure in .probe() to set driver data before the function is left to
> make it possible in .remove() to undo the actions done.
> 
> This fixes a potential memory leak and stops returning an error code in
> .remove() that is ignored by the driver core anyhow.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This problem is still real and the patch not applied. Who feels
responsible to pick this up?

> ---
>  arch/arm/mach-mmp/sram.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c
> index 6794e2db1ad5..ecc46c31004f 100644
> --- a/arch/arm/mach-mmp/sram.c
> +++ b/arch/arm/mach-mmp/sram.c
> @@ -72,6 +72,8 @@ static int sram_probe(struct platform_device *pdev)
>  	if (!info)
>  		return -ENOMEM;
>  
> +	platform_set_drvdata(pdev, info);
> +
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (res == NULL) {
>  		dev_err(&pdev->dev, "no memory resource defined\n");
> @@ -107,8 +109,6 @@ static int sram_probe(struct platform_device *pdev)
>  	list_add(&info->node, &sram_bank_list);
>  	mutex_unlock(&sram_lock);
>  
> -	platform_set_drvdata(pdev, info);
> -
>  	dev_info(&pdev->dev, "initialized\n");
>  	return 0;
>  
> @@ -127,17 +127,19 @@ static int sram_remove(struct platform_device *pdev)
>  	struct sram_bank_info *info;
>  
>  	info = platform_get_drvdata(pdev);
> -	if (info == NULL)
> -		return -ENODEV;
>  
> -	mutex_lock(&sram_lock);
> -	list_del(&info->node);
> -	mutex_unlock(&sram_lock);
> +	if (info->sram_size) {
> +		mutex_lock(&sram_lock);
> +		list_del(&info->node);
> +		mutex_unlock(&sram_lock);
> +
> +		gen_pool_destroy(info->gpool);
> +		iounmap(info->sram_virt);
> +		kfree(info->pool_name);
> +	}
>  
> -	gen_pool_destroy(info->gpool);
> -	iounmap(info->sram_virt);
> -	kfree(info->pool_name);
>  	kfree(info);
> +
>  	return 0;
>  }

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH] ARM: mmp: Fix failure to remove sram device

[Arnd Bergmann]

On Mon, Feb 21, 2022 at 1:14 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> [adding soc@kernel.org to Cc]
>
> On Mon, Jul 26, 2021 at 10:01:58PM +0200, Uwe Kleine-König wrote:
> > Make sure in .probe() to set driver data before the function is left to
> > make it possible in .remove() to undo the actions done.
> >
> > This fixes a potential memory leak and stops returning an error code in
> > .remove() that is ignored by the driver core anyhow.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> This problem is still real and the patch not applied. Who feels
> responsible to pick this up?

I've picked it up manually now, thank you for looping us in.

Note that I almost missed it again though, if you really want my attention,
send the patch to:soc@kernel.org so it makes it into patchwork. At that
point, I'll have to either apply it or actively discard the patch from there.

       Arnd

Re: [PATCH] ARM: mmp: Fix failure to remove sram device

[Arnd Bergmann]

On Mon, Feb 21, 2022 at 1:14 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> [adding soc@kernel.org to Cc]
>
> On Mon, Jul 26, 2021 at 10:01:58PM +0200, Uwe Kleine-König wrote:
> > Make sure in .probe() to set driver data before the function is left to
> > make it possible in .remove() to undo the actions done.
> >
> > This fixes a potential memory leak and stops returning an error code in
> > .remove() that is ignored by the driver core anyhow.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> This problem is still real and the patch not applied. Who feels
> responsible to pick this up?

I've picked it up manually now, thank you for looping us in.

Note that I almost missed it again though, if you really want my attention,
send the patch to:soc@kernel.org so it makes it into patchwork. At that
point, I'll have to either apply it or actively discard the patch from there.

       Arnd

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