| Message ID | 20240601113502.2709597-1-yangyingliang@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: core: fix possible memory leak in error path in pinctrl_enable() | expand |
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@huawei.com> > --- > 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);
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); + } + return error; } @@ -2283,6 +2290,7 @@ struct pinctrl_dev *devm_pinctrl_register(struct device *dev, return pctldev; } + pctldev->devm_allocated = true; *ptr = pctldev; devres_add(dev, ptr); @@ -2319,6 +2327,7 @@ int devm_pinctrl_register_and_init(struct device *dev, return error; } + (*pctldev)->devm_allocated = true; *ptr = *pctldev; devres_add(dev, ptr); diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h index 837fd5bd903d..bf57ce4fe927 100644 --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -72,6 +72,7 @@ struct pinctrl_dev { #ifdef CONFIG_DEBUG_FS struct dentry *device_root; #endif + bool devm_allocated; }; /**
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@huawei.com> --- drivers/pinctrl/core.c | 9 +++++++++ drivers/pinctrl/core.h | 1 + 2 files changed, 10 insertions(+)