[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state

[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state

[YueHaibing]

fixes following Smatch static check warning:

 drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
 warn: passing devm_ allocated variable to kfree. 'pctrl->functions'

As we will be calling krealloc() on pointer 'pctrl->functions', which means
kfree() will be called in there, devm_kzalloc() shouldn't be used with
the allocation in the first place.  Fix the warning by calling kcalloc()
and managing the free procedure in error path on our own.

Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell Berlin SoCs")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: free pctrl->functions instead of function as Jisheng Zhang suggested
---
 drivers/pinctrl/berlin/berlin.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index d6d183e..b5903ff 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 	}
 
 	/* we will reallocate later */
-	pctrl->functions = devm_kcalloc(&pdev->dev,
-					max_functions,
-					sizeof(*pctrl->functions),
-					GFP_KERNEL);
+	pctrl->functions = kcalloc(max_functions,
+				   sizeof(*pctrl->functions), GFP_KERNEL);
 	if (!pctrl->functions)
 		return -ENOMEM;
 
@@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 				function++;
 			}
 
-			if (!found)
+			if (!found) {
+				kfree( );
 				return -EINVAL;
+			}
 
 			if (!function->groups) {
 				function->groups =
@@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 						     sizeof(char *),
 						     GFP_KERNEL);
 
-				if (!function->groups)
+				if (!function->groups) {
+					kfree(pctrl->functions);
 					return -ENOMEM;
+				}
 			}
 
 			groups = function->groups;
-- 
2.7.0

Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state

[YueHaibing]

Sorry, I send a wrong patch, pls ignore this.

On 2018/8/1 13:02, YueHaibing wrote:
> fixes following Smatch static check warning:
> 
>  drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
>  warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
> 
> As we will be calling krealloc() on pointer 'pctrl->functions', which means
> kfree() will be called in there, devm_kzalloc() shouldn't be used with
> the allocation in the first place.  Fix the warning by calling kcalloc()
> and managing the free procedure in error path on our own.
> 
> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell Berlin SoCs")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> v2: free pctrl->functions instead of function as Jisheng Zhang suggested
> ---
>  drivers/pinctrl/berlin/berlin.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
> index d6d183e..b5903ff 100644
> --- a/drivers/pinctrl/berlin/berlin.c
> +++ b/drivers/pinctrl/berlin/berlin.c
> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
>  	}
>  
>  	/* we will reallocate later */
> -	pctrl->functions = devm_kcalloc(&pdev->dev,
> -					max_functions,
> -					sizeof(*pctrl->functions),
> -					GFP_KERNEL);
> +	pctrl->functions = kcalloc(max_functions,
> +				   sizeof(*pctrl->functions), GFP_KERNEL);
>  	if (!pctrl->functions)
>  		return -ENOMEM;
>  
> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
>  				function++;
>  			}
>  
> -			if (!found)
> +			if (!found) {
> +				kfree( );
>  				return -EINVAL;
> +			}
>  
>  			if (!function->groups) {
>  				function->groups =
> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
>  						     sizeof(char *),
>  						     GFP_KERNEL);
>  
> -				if (!function->groups)
> +				if (!function->groups) {
> +					kfree(pctrl->functions);
>  					return -ENOMEM;
> +				}
>  			}
>  
>  			groups = function->groups;
> 

Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4023 bytes --]

Hi YueHaibing,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.18-rc7 next-20180731]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/YueHaibing/pinctrl-berlin-fix-pctrl-functions-allocation-in-berlin_pinctrl_build_state/20180801-131813
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/pinctrl/berlin/berlin.c: In function 'berlin_pinctrl_build_state':
>> drivers/pinctrl/berlin/berlin.c:259:5: error: too few arguments to function 'kfree'
        kfree( );
        ^~~~~
   In file included from drivers/pinctrl/berlin/berlin.c:20:
   include/linux/slab.h:183:6: note: declared here
    void kfree(const void *);
         ^~~~~

vim +/kfree +259 drivers/pinctrl/berlin/berlin.c

   202	
   203	static int berlin_pinctrl_build_state(struct platform_device *pdev)
   204	{
   205		struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev);
   206		const struct berlin_desc_group *desc_group;
   207		const struct berlin_desc_function *desc_function;
   208		int i, max_functions = 0;
   209	
   210		pctrl->nfunctions = 0;
   211	
   212		for (i = 0; i < pctrl->desc->ngroups; i++) {
   213			desc_group = pctrl->desc->groups + i;
   214			/* compute the maxiumum number of functions a group can have */
   215			max_functions += 1 << (desc_group->bit_width + 1);
   216		}
   217	
   218		/* we will reallocate later */
   219		pctrl->functions = kcalloc(max_functions,
   220					   sizeof(*pctrl->functions), GFP_KERNEL);
   221		if (!pctrl->functions)
   222			return -ENOMEM;
   223	
   224		/* register all functions */
   225		for (i = 0; i < pctrl->desc->ngroups; i++) {
   226			desc_group = pctrl->desc->groups + i;
   227			desc_function = desc_group->functions;
   228	
   229			while (desc_function->name) {
   230				berlin_pinctrl_add_function(pctrl, desc_function->name);
   231				desc_function++;
   232			}
   233		}
   234	
   235		pctrl->functions = krealloc(pctrl->functions,
   236					    pctrl->nfunctions * sizeof(*pctrl->functions),
   237					    GFP_KERNEL);
   238	
   239		/* map functions to theirs groups */
   240		for (i = 0; i < pctrl->desc->ngroups; i++) {
   241			desc_group = pctrl->desc->groups + i;
   242			desc_function = desc_group->functions;
   243	
   244			while (desc_function->name) {
   245				struct berlin_pinctrl_function
   246					*function = pctrl->functions;
   247				const char **groups;
   248				bool found = false;
   249	
   250				while (function->name) {
   251					if (!strcmp(desc_function->name, function->name)) {
   252						found = true;
   253						break;
   254					}
   255					function++;
   256				}
   257	
   258				if (!found) {
 > 259					kfree( );
   260					return -EINVAL;
   261				}
   262	
   263				if (!function->groups) {
   264					function->groups =
   265						devm_kcalloc(&pdev->dev,
   266							     function->ngroups,
   267							     sizeof(char *),
   268							     GFP_KERNEL);
   269	
   270					if (!function->groups) {
   271						kfree(pctrl->functions);
   272						return -ENOMEM;
   273					}
   274				}
   275	
   276				groups = function->groups;
   277				while (*groups)
   278					groups++;
   279	
   280				*groups = desc_group->name;
   281	
   282				desc_function++;
   283			}
   284		}
   285	
   286		return 0;
   287	}
   288	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54170 bytes --]