Re: [PATCH v2] pinctrl:sunplus: Add check for kmalloc

[andy . shevchenko]

Sun, May 28, 2023 at 08:34:37PM +0800, Wells Lu kirjoitti:
> Fix Smatch static checker warning:
> potential null dereference 'configs'. (kmalloc returns null)

> Changes in v2:
> 1. Add free allocated memory before returned -ENOMEM.
> 2. Add call of_node_put() before returned -ENOMEM.

The placeholder for this is after the cutter '---' line.
I think it may be fixed during application by Linus W.

Otherwise LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

> Fixes: aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")
> Signed-off-by: Wells Lu <wellslutw@xxxxxxxxx>
> ---
>  drivers/pinctrl/sunplus/sppctl.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunplus/sppctl.c b/drivers/pinctrl/sunplus/sppctl.c
> index 6bbbab3..e91ce5b 100644
> --- a/drivers/pinctrl/sunplus/sppctl.c
> +++ b/drivers/pinctrl/sunplus/sppctl.c
> @@ -834,11 +834,6 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>  	int i, size = 0;
>  
>  	list = of_get_property(np_config, "sunplus,pins", &size);
> -
> -	if (nmG <= 0)
> -		nmG = 0;
> -
> -	parent = of_get_parent(np_config);
>  	*num_maps = size / sizeof(*list);
>  
>  	/*
> @@ -866,10 +861,14 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>  		}
>  	}
>  
> +	if (nmG <= 0)
> +		nmG = 0;
> +
>  	*map = kcalloc(*num_maps + nmG, sizeof(**map), GFP_KERNEL);
> -	if (*map == NULL)
> +	if (!(*map))
>  		return -ENOMEM;
>  
> +	parent = of_get_parent(np_config);
>  	for (i = 0; i < (*num_maps); i++) {
>  		dt_pin = be32_to_cpu(list[i]);
>  		pin_num = FIELD_GET(GENMASK(31, 24), dt_pin);
> @@ -883,6 +882,8 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>  			(*map)[i].data.configs.num_configs = 1;
>  			(*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, pin_num);
>  			configs = kmalloc(sizeof(*configs), GFP_KERNEL);
> +			if (!configs)
> +				goto sppctl_map_err;
>  			*configs = FIELD_GET(GENMASK(7, 0), dt_pin);
>  			(*map)[i].data.configs.configs = configs;
>  
> @@ -896,6 +897,8 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>  			(*map)[i].data.configs.num_configs = 1;
>  			(*map)[i].data.configs.group_or_pin = pin_get_name(pctldev, pin_num);
>  			configs = kmalloc(sizeof(*configs), GFP_KERNEL);
> +			if (!configs)
> +				goto sppctl_map_err;
>  			*configs = SPPCTL_IOP_CONFIGS;
>  			(*map)[i].data.configs.configs = configs;
>  
> @@ -965,6 +968,15 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>  	of_node_put(parent);
>  	dev_dbg(pctldev->dev, "%d pins mapped\n", *num_maps);
>  	return 0;
> +
> +sppctl_map_err:
> +	for (i = 0; i < (*num_maps); i++)
> +		if (((*map)[i].type == PIN_MAP_TYPE_CONFIGS_PIN) &&
> +		    (*map)[i].data.configs.configs)
> +			kfree((*map)[i].data.configs.configs);
> +	kfree(*map);
> +	of_node_put(parent);
> +	return -ENOMEM;
>  }
>  
>  static const struct pinctrl_ops sppctl_pctl_ops = {
> -- 
> 2.7.4
> 

-- 
With Best Regards,
Andy Shevchenko