* [PATCH v2] pinctrl:sunplus: Add check for kmalloc
@ 2023-05-28 12:34 Wells Lu
2023-05-28 15:02 ` andy.shevchenko
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Wells Lu @ 2023-05-28 12:34 UTC (permalink / raw)
To: linus.walleij, linux-gpio, linux-kernel; +Cc: wells.lu, Wells Lu
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 <wellslutw@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl:sunplus: Add check for kmalloc
2023-05-28 12:34 [PATCH v2] pinctrl:sunplus: Add check for kmalloc Wells Lu
@ 2023-05-28 15:02 ` andy.shevchenko
2023-05-30 11:48 ` Linus Walleij
[not found] ` <9ace5c4c-5e17-4207-5a02-6a47ba0aee22@web.de>
2 siblings, 0 replies; 5+ messages in thread
From: andy.shevchenko @ 2023-05-28 15:02 UTC (permalink / raw)
To: Wells Lu; +Cc: linus.walleij, linux-gpio, linux-kernel, wells.lu
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@gmail.com>
> Fixes: aa74c44be19c ("pinctrl: Add driver for Sunplus SP7021")
> Signed-off-by: Wells Lu <wellslutw@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] pinctrl:sunplus: Add check for kmalloc
2023-05-28 12:34 [PATCH v2] pinctrl:sunplus: Add check for kmalloc Wells Lu
2023-05-28 15:02 ` andy.shevchenko
@ 2023-05-30 11:48 ` Linus Walleij
[not found] ` <9ace5c4c-5e17-4207-5a02-6a47ba0aee22@web.de>
2 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2023-05-30 11:48 UTC (permalink / raw)
To: Wells Lu; +Cc: linux-gpio, linux-kernel, wells.lu
On Sun, May 28, 2023 at 2:35 PM Wells Lu <wellslutw@gmail.com> 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 <wellslutw@gmail.com>
Patch applied as non-urgent fix, dropped the changelog as pointed
out by Andy.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pinctrl: sunplus: Delete an unnecessary check before kfree() in sppctl_dt_node_to_map()
[not found] ` <9ace5c4c-5e17-4207-5a02-6a47ba0aee22@web.de>
@ 2023-06-06 14:39 ` Andy Shevchenko
2023-06-09 7:32 ` Linus Walleij
1 sibling, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2023-06-06 14:39 UTC (permalink / raw)
To: Markus Elfring, Bartosz Golaszewski
Cc: kernel-janitors, linux-gpio, linux-arm-kernel, Dvorkin Dmitry,
Linus Walleij, Wells Lu, LKML, cocci, Wells Lu
On Tue, Jun 6, 2023 at 4:26 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 6 Jun 2023 15:00:18 +0200
You need to utilize what MAINTAINERS file has.
> It can be known that the function “kfree” performs a null pointer check
> for its input parameter.
> It is therefore not needed to repeat such a check before its call.
>
> Thus remove a redundant pointer check.
Seems reasonable to me.
FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/pinctrl/sunplus/sppctl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sunplus/sppctl.c b/drivers/pinctrl/sunplus/sppctl.c
> index e91ce5b5d559..150996949ede 100644
> --- a/drivers/pinctrl/sunplus/sppctl.c
> +++ b/drivers/pinctrl/sunplus/sppctl.c
> @@ -971,8 +971,7 @@ static int sppctl_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node
>
> sppctl_map_err:
> for (i = 0; i < (*num_maps); i++)
> - if (((*map)[i].type == PIN_MAP_TYPE_CONFIGS_PIN) &&
> - (*map)[i].data.configs.configs)
> + if ((*map)[i].type == PIN_MAP_TYPE_CONFIGS_PIN)
> kfree((*map)[i].data.configs.configs);
> kfree(*map);
> of_node_put(parent);
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] pinctrl: sunplus: Delete an unnecessary check before kfree() in sppctl_dt_node_to_map()
[not found] ` <9ace5c4c-5e17-4207-5a02-6a47ba0aee22@web.de>
2023-06-06 14:39 ` [PATCH] pinctrl: sunplus: Delete an unnecessary check before kfree() in sppctl_dt_node_to_map() Andy Shevchenko
@ 2023-06-09 7:32 ` Linus Walleij
1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2023-06-09 7:32 UTC (permalink / raw)
To: Markus Elfring, Lu Hongfei
Cc: kernel-janitors, linux-gpio, linux-arm-kernel, Andy Shevchenko,
Dvorkin Dmitry, Wells Lu, LKML, cocci, Wells Lu
On Tue, Jun 6, 2023 at 3:26 PM Markus Elfring <Markus.Elfring@web.de> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 6 Jun 2023 15:00:18 +0200
>
> It can be known that the function “kfree” performs a null pointer check
> for its input parameter.
> It is therefore not needed to repeat such a check before its call.
>
> Thus remove a redundant pointer check.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Lu Hongfei sent a patch changing the code around this location
so I rebased it manually and applied, check the result, thanks!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-09 7:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-28 12:34 [PATCH v2] pinctrl:sunplus: Add check for kmalloc Wells Lu
2023-05-28 15:02 ` andy.shevchenko
2023-05-30 11:48 ` Linus Walleij
[not found] ` <9ace5c4c-5e17-4207-5a02-6a47ba0aee22@web.de>
2023-06-06 14:39 ` [PATCH] pinctrl: sunplus: Delete an unnecessary check before kfree() in sppctl_dt_node_to_map() Andy Shevchenko
2023-06-09 7:32 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).