Re: [PATCH] pinctrl: core: fix possible memory leak in error path in pinctrl_enable()

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Sat, Jun 01, 2024 at 07:35:02PM +0800, Yang Yingliang wrote:
> In devm_pinctrl_register(), if pinctrl_enable() fails in pinctrl_register(),
> the "pctldev" has not been added to dev resources, so devm_pinctrl_dev_release()
> can not be called, it leads memory leak.
> 
> And some driver calls pinctrl_register_and_init() which is not devm_ managed,
> it also leads memory leak if pinctrl_enable() fails.
> 
> To fix this, add a flag devm_allocated to struct pinctrl_dev, free the memory
> by checking this flag.
> 
> Fixes: 5038a66dad01 ("pinctrl: core: delete incorrect free in pinctrl_enable()")
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> ---
>  drivers/pinctrl/core.c | 9 +++++++++
>  drivers/pinctrl/core.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index cffeb869130d..374c36f5c759 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -2125,6 +2125,13 @@ int pinctrl_enable(struct pinctrl_dev *pctldev)
>  	error = pinctrl_claim_hogs(pctldev);
>  	if (error) {
>  		dev_err(pctldev->dev, "could not claim hogs: %i\n", error);
> +		if (!pctldev->devm_allocated) {
> +			pinctrl_free_pindescs(pctldev, pctldev->desc->pins,
> +					      pctldev->desc->npins);
> +			mutex_destroy(&pctldev->mutex);
> +			kfree(pctldev);
> +		}

The other thing is, this should be done in a function.

ti_iodelay_probe() would have to call uninit as well.  The error
handling in ti_iodelay_probe() was already messed up.  They probably
assumed that pinctrl_enable() would do the cleanup so they forgot to
call of_node_put(np).

regards,
dan carpenter

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 9fcb9d913556..704541385397 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -2090,6 +2090,13 @@ pinctrl_init_controller(struct pinctrl_desc *pctldesc, struct device *dev,
 	return ERR_PTR(ret);
 }
 
+void pinctrl_uninit_controller(struct pinctrl_dev *pctldev)
+{
+	pinctrl_free_pindescs(pctldev, pctldev->desc->pins, pctldev->desc->npins);
+	mutex_destroy(&pctldev->mutex);
+	kfree(pctldev);
+}
+
 static int pinctrl_claim_hogs(struct pinctrl_dev *pctldev)
 {
 	pctldev->p = create_pinctrl(pctldev->dev, pctldev);