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.
Fixes: aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")
Signed-off-by: Wells Lu <[email protected]>
---
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
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 <[email protected]>
> Fixes: aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")
> Signed-off-by: Wells Lu <[email protected]>
> ---
> 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
On Sun, May 28, 2023 at 2:35 PM Wells Lu <[email protected]> wrote:
> 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.
>
> Fixes: aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")
> Signed-off-by: Wells Lu <[email protected]>
Patch applied as non-urgent fix, dropped the changelog as pointed
out by Andy.
Yours,
Linus Walleij