| Message ID | 20221129113410.1555592-1-cuigaosheng1@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] pinctrl: thunderbay: fix possible memory leak in thunderbay_build_functions() | expand |
On 29.11.2022 12:34, Gaosheng Cui wrote: > The thunderbay_add_functions() will free memory of thunderbay_funcs > when everything is ok, but thunderbay_funcs will not be freed when > thunderbay_add_functions() fails, then there will be a memory leak, > so add kfree() when thunderbay_add_functions() fails to fix it. > > Fixes: 12422af8194d ("pinctrl: Add Intel Thunder Bay pinctrl driver") > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > v2: > - Free the memory in thunderbay_build_functions, and update the commit > message and the fixes tag, thanks! > drivers/pinctrl/pinctrl-thunderbay.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/pinctrl-thunderbay.c b/drivers/pinctrl/pinctrl-thunderbay.c > index 9328b17485cf..84b64c3217a5 100644 > --- a/drivers/pinctrl/pinctrl-thunderbay.c > +++ b/drivers/pinctrl/pinctrl-thunderbay.c > @@ -817,6 +817,7 @@ static int thunderbay_build_functions(struct thunderbay_pinctrl *tpc) > struct function_desc *thunderbay_funcs; > void *ptr; > int pin; > + int ret; > > /* > * Allocate maximum possible number of functions. Assume every pin > @@ -860,7 +861,13 @@ static int thunderbay_build_functions(struct thunderbay_pinctrl *tpc) > return -ENOMEM; > > thunderbay_funcs = ptr; > - return thunderbay_add_functions(tpc, thunderbay_funcs); > + ret = thunderbay_add_functions(tpc, thunderbay_funcs); > + if (ret < 0) { > + kfree(thunderbay_funcs); > + return ret; > + } > + > + return 0; > } > > static int thunderbay_pinconf_set_tristate(struct thunderbay_pinctrl *tpc, Sorry, I probably wasn't precise enough. This patch still leaves *existing* kfree(funcs); in the thunderbay_add_functions(). Maybe you can get rid of that one and just handle freeing in the thunderbay_build_functions()?
> > This patch still leaves *existing* kfree(funcs); in the > thunderbay_add_functions(). Maybe you can get rid of that one and just > handle freeing in the thunderbay_build_functions()? Thanks for taking time to review this patch, I have updated this patch and submitted it, If you have any suggestions, please let me know, thanks again! On 2022/11/29 19:36, Rafał Miłecki wrote: > > > On 29.11.2022 12:34, Gaosheng Cui wrote: >> The thunderbay_add_functions() will free memory of thunderbay_funcs >> when everything is ok, but thunderbay_funcs will not be freed when >> thunderbay_add_functions() fails, then there will be a memory leak, >> so add kfree() when thunderbay_add_functions() fails to fix it. >> >> Fixes: 12422af8194d ("pinctrl: Add Intel Thunder Bay pinctrl driver") >> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> >> --- >> v2: >> - Free the memory in thunderbay_build_functions, and update the commit >> message and the fixes tag, thanks! >> drivers/pinctrl/pinctrl-thunderbay.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pinctrl/pinctrl-thunderbay.c >> b/drivers/pinctrl/pinctrl-thunderbay.c >> index 9328b17485cf..84b64c3217a5 100644 >> --- a/drivers/pinctrl/pinctrl-thunderbay.c >> +++ b/drivers/pinctrl/pinctrl-thunderbay.c >> @@ -817,6 +817,7 @@ static int thunderbay_build_functions(struct >> thunderbay_pinctrl *tpc) >> struct function_desc *thunderbay_funcs; >> void *ptr; >> int pin; >> + int ret; >> /* >> * Allocate maximum possible number of functions. Assume every pin >> @@ -860,7 +861,13 @@ static int thunderbay_build_functions(struct >> thunderbay_pinctrl *tpc) >> return -ENOMEM; >> thunderbay_funcs = ptr; >> - return thunderbay_add_functions(tpc, thunderbay_funcs); >> + ret = thunderbay_add_functions(tpc, thunderbay_funcs); >> + if (ret < 0) { >> + kfree(thunderbay_funcs); >> + return ret; >> + } >> + >> + return 0; >> } >> static int thunderbay_pinconf_set_tristate(struct >> thunderbay_pinctrl *tpc, > > Sorry, I probably wasn't precise enough. > > This patch still leaves *existing* kfree(funcs); in the > thunderbay_add_functions(). Maybe you can get rid of that one and just > handle freeing in the thunderbay_build_functions()? > > .
diff --git a/drivers/pinctrl/pinctrl-thunderbay.c b/drivers/pinctrl/pinctrl-thunderbay.c index 9328b17485cf..84b64c3217a5 100644 --- a/drivers/pinctrl/pinctrl-thunderbay.c +++ b/drivers/pinctrl/pinctrl-thunderbay.c @@ -817,6 +817,7 @@ static int thunderbay_build_functions(struct thunderbay_pinctrl *tpc) struct function_desc *thunderbay_funcs; void *ptr; int pin; + int ret; /* * Allocate maximum possible number of functions. Assume every pin @@ -860,7 +861,13 @@ static int thunderbay_build_functions(struct thunderbay_pinctrl *tpc) return -ENOMEM; thunderbay_funcs = ptr; - return thunderbay_add_functions(tpc, thunderbay_funcs); + ret = thunderbay_add_functions(tpc, thunderbay_funcs); + if (ret < 0) { + kfree(thunderbay_funcs); + return ret; + } + + return 0; } static int thunderbay_pinconf_set_tristate(struct thunderbay_pinctrl *tpc,
The thunderbay_add_functions() will free memory of thunderbay_funcs when everything is ok, but thunderbay_funcs will not be freed when thunderbay_add_functions() fails, then there will be a memory leak, so add kfree() when thunderbay_add_functions() fails to fix it. Fixes: 12422af8194d ("pinctrl: Add Intel Thunder Bay pinctrl driver") Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> --- v2: - Free the memory in thunderbay_build_functions, and update the commit message and the fixes tag, thanks! drivers/pinctrl/pinctrl-thunderbay.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)