diff mbox series

pinctrl: sunxi: fix regulator leak in sunxi_pmx_request() error path

Message ID 20260504-sunxi-v1-1-c0fd0df52776@gmail.com (mailing list archive)
State New
Headers
Series pinctrl: sunxi: fix regulator leak in sunxi_pmx_request() error path |

Commit Message

Felix Gu May 4, 2026, 2:53 p.m. UTC 
In the error path of sunxi_pmx_request(), the code calls
regulator_put(s_reg->regulator) to release the regulator. However,
s_reg->regulator is only assigned after a successful regulator_enable().
This causes a memory leak: the regulator obtained via regulator_get()
is never properly released when regulator_enable() fails.

Fixes: dc1445584177 ("pinctrl: sunxi: Fix and simplify pin bank regulator handling")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: b9303e6bff706758c167af686b5315ad00233bf8
change-id: 20260504-sunxi-cda91661c181

Best regards,

Comments

Andre Przywara May 4, 2026, 11:33 p.m. UTC | #1 
On Mon, 04 May 2026 22:53:26 +0800
Felix Gu <ustc.gu@gmail.com> wrote:

> In the error path of sunxi_pmx_request(), the code calls
> regulator_put(s_reg->regulator) to release the regulator. However,
> s_reg->regulator is only assigned after a successful regulator_enable().
> This causes a memory leak: the regulator obtained via regulator_get()
> is never properly released when regulator_enable() fails.

Yes, that's a correct observation. The fix looks alright, though I
wonder if we should drop the "goto" here altogether, since there is only
one caller, and the code would look better like this:

	ret = regulator_enable(reg);
	if (ret) {
		dev_err(pctl->dev, ...
		regulator_put(reg);
		return ret;
        }
...

> Fixes: dc1445584177 ("pinctrl: sunxi: Fix and simplify pin bank regulator handling")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

Regardless, the fix is correct, so:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index d3042e0c9712..25489beeb312 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -925,7 +925,7 @@ static int sunxi_pmx_request(struct pinctrl_dev *pctldev, unsigned offset)
>  	return 0;
>  
>  out:
> -	regulator_put(s_reg->regulator);
> +	regulator_put(reg);
>  
>  	return ret;
>  }
> 
> ---
> base-commit: b9303e6bff706758c167af686b5315ad00233bf8
> change-id: 20260504-sunxi-cda91661c181
> 
> Best regards,
Linus Walleij May 5, 2026, 9:26 a.m. UTC | #2 
On Mon, May 4, 2026 at 4:53 PM Felix Gu <ustc.gu@gmail.com> wrote:

> In the error path of sunxi_pmx_request(), the code calls
> regulator_put(s_reg->regulator) to release the regulator. However,
> s_reg->regulator is only assigned after a successful regulator_enable().
> This causes a memory leak: the regulator obtained via regulator_get()
> is never properly released when regulator_enable() fails.
>
> Fixes: dc1445584177 ("pinctrl: sunxi: Fix and simplify pin bank regulator handling")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

Patch applied as nonurgent fix!

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index d3042e0c9712..25489beeb312 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -925,7 +925,7 @@  static int sunxi_pmx_request(struct pinctrl_dev *pctldev, unsigned offset)
 	return 0;
 
 out:
-	regulator_put(s_reg->regulator);
+	regulator_put(reg);
 
 	return ret;
 }