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);
> +		}

Why not do this free in pinctrl_register()?  It's a layering violation
to do it here.  It's not where you expect the free to be.  It will
just lead to double free bugs.

regards,
dan carpenter