[PATCH 0/3] pinctrl: check memory returned by devm_kasprintf()

[PATCH 0/3] pinctrl: check memory returned by devm_kasprintf()

[Claudiu Beznea]

Hi,

While browsing some code I noticed that there are places where pointer
returned by devm_kasprintf() or kasprintf() is not checked. Thus I've
tooked the chance and fixed this (by updating kmerr.cocci script,
changes published at [1]).

Thank you,
Claudiu Beznea

[1] https://lore.kernel.org/all/20230530074044.1603426-1-claudiu.beznea@microchip.com/

Claudiu Beznea (3):
  pinctrl: mcp23s08: check return value of {devm_}kasprintf()
  pinctrl: microchip-sgpio: check return value of devm_kasprintf()
  pinctrl: at91-pio4: check return value of devm_kasprintf()

 drivers/pinctrl/pinctrl-at91-pio4.c       | 2 ++
 drivers/pinctrl/pinctrl-mcp23s08_spi.c    | 3 +++
 drivers/pinctrl/pinctrl-microchip-sgpio.c | 3 +++
 3 files changed, 8 insertions(+)

-- 
2.34.1

[PATCH 1/3] pinctrl: mcp23s08: check return value of devm_kasprintf()

[Claudiu Beznea]

devm_kasprintf() returns a pointer to dynamically allocated memory.
Pointer could be NULL in case allocation fails. Check pointer validity.
Identified with coccinelle (kmerr.cocci script).

Fixes: 0f04a81784fe ("pinctrl: mcp23s08: Split to three parts: core, I²C, SPI")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pinctrl/pinctrl-mcp23s08_spi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08_spi.c b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
index 9ae10318f6f3..4013ffbfa282 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08_spi.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08_spi.c
@@ -119,6 +119,9 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
 		return -EINVAL;
 	}
 
+	if (!name || !mcp->chip.label)
+		return -ENOMEM;
+
 	copy = devm_kmemdup(dev, config, sizeof(*config), GFP_KERNEL);
 	if (!copy)
 		return -ENOMEM;
-- 
2.34.1

Re: [PATCH 1/3] pinctrl: mcp23s08: check return value of devm_kasprintf()

[Andy Shevchenko]

On Thu, Jun 15, 2023 at 01:53:31PM +0300, Claudiu Beznea wrote:
> devm_kasprintf() returns a pointer to dynamically allocated memory.
> Pointer could be NULL in case allocation fails. Check pointer validity.
> Identified with coccinelle (kmerr.cocci script).

Not that this may happen, but okay.

...

> @@ -119,6 +119,9 @@ static int mcp23s08_spi_regmap_init(struct mcp23s08 *mcp, struct device *dev,
>  		return -EINVAL;
>  	}
>  
> +	if (!name || !mcp->chip.label)
> +		return -ENOMEM;

I prefer to see 4 independent checks for each of the devm_kasprintf() calls.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 2/3] pinctrl: microchip-sgpio: check return value of devm_kasprintf()

[Claudiu Beznea]

devm_kasprintf() returns a pointer to dynamically allocated memory.
Pointer could be NULL in case allocation fails. Check pointer validity.
Identified with coccinelle (kmerr.cocci script).

Fixes: 7e5ea974e61c ("pinctrl: pinctrl-microchip-sgpio: Add pinctrl driver for Microsemi Serial GPIO")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pinctrl/pinctrl-microchip-sgpio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c
index 4794602316e7..666d8b7cdbad 100644
--- a/drivers/pinctrl/pinctrl-microchip-sgpio.c
+++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c
@@ -818,6 +818,9 @@ static int microchip_sgpio_register_bank(struct device *dev,
 	pctl_desc->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%sput",
 					 dev_name(dev),
 					 bank->is_input ? "in" : "out");
+	if (!pctl_desc->name)
+		return -ENOMEM;
+
 	pctl_desc->pctlops = &sgpio_pctl_ops;
 	pctl_desc->pmxops = &sgpio_pmx_ops;
 	pctl_desc->confops = &sgpio_confops;
-- 
2.34.1

Re: [PATCH 2/3] pinctrl: microchip-sgpio: check return value of devm_kasprintf()

[Andy Shevchenko]

On Thu, Jun 15, 2023 at 01:53:32PM +0300, Claudiu Beznea wrote:
> devm_kasprintf() returns a pointer to dynamically allocated memory.
> Pointer could be NULL in case allocation fails. Check pointer validity.
> Identified with coccinelle (kmerr.cocci script).

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Fixes: 7e5ea974e61c ("pinctrl: pinctrl-microchip-sgpio: Add pinctrl driver for Microsemi Serial GPIO")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/pinctrl/pinctrl-microchip-sgpio.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c
> index 4794602316e7..666d8b7cdbad 100644
> --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c
> +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c
> @@ -818,6 +818,9 @@ static int microchip_sgpio_register_bank(struct device *dev,
>  	pctl_desc->name = devm_kasprintf(dev, GFP_KERNEL, "%s-%sput",
>  					 dev_name(dev),
>  					 bank->is_input ? "in" : "out");
> +	if (!pctl_desc->name)
> +		return -ENOMEM;
> +
>  	pctl_desc->pctlops = &sgpio_pctl_ops;
>  	pctl_desc->pmxops = &sgpio_pmx_ops;
>  	pctl_desc->confops = &sgpio_confops;
> -- 
> 2.34.1
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/3] pinctrl: at91-pio4: check return value of devm_kasprintf()

[Claudiu Beznea]

devm_kasprintf() returns a pointer to dynamically allocated memory.
Pointer could be NULL in case allocation fails. Check pointer validity.
Identified with coccinelle (kmerr.cocci script).

Fixes: 776180848b57 ("pinctrl: introduce driver for Atmel PIO4 controller")
Depends-on: 1c4e5c470a56 ("pinctrl: at91: use devm_kasprintf() to avoid potential leaks")
Depends-on: 5a8f9cf269e8 ("pinctrl: at91-pio4: use proper format specifier for unsigned int")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/pinctrl/pinctrl-at91-pio4.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index e40487be2038..fc7d7155bb8d 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -1146,6 +1146,8 @@ static int atmel_pinctrl_probe(struct platform_device *pdev)
 		/* Pin naming convention: P(bank_name)(bank_pin_number). */
 		pin_desc[i].name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "P%c%u",
 						  bank + 'A', line);
+		if (!pin_desc[i].name)
+			return -ENOMEM;
 
 		group->name = group_names[i] = pin_desc[i].name;
 		group->pin = pin_desc[i].number;
-- 
2.34.1

Re: [PATCH 3/3] pinctrl: at91-pio4: check return value of devm_kasprintf()

[Andy Shevchenko]

On Thu, Jun 15, 2023 at 01:53:33PM +0300, Claudiu Beznea wrote:
> devm_kasprintf() returns a pointer to dynamically allocated memory.
> Pointer could be NULL in case allocation fails. Check pointer validity.
> Identified with coccinelle (kmerr.cocci script).

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Fixes: 776180848b57 ("pinctrl: introduce driver for Atmel PIO4 controller")
> Depends-on: 1c4e5c470a56 ("pinctrl: at91: use devm_kasprintf() to avoid potential leaks")
> Depends-on: 5a8f9cf269e8 ("pinctrl: at91-pio4: use proper format specifier for unsigned int")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/pinctrl/pinctrl-at91-pio4.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
> index e40487be2038..fc7d7155bb8d 100644
> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
> @@ -1146,6 +1146,8 @@ static int atmel_pinctrl_probe(struct platform_device *pdev)
>  		/* Pin naming convention: P(bank_name)(bank_pin_number). */
>  		pin_desc[i].name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "P%c%u",
>  						  bank + 'A', line);
> +		if (!pin_desc[i].name)
> +			return -ENOMEM;
>  
>  		group->name = group_names[i] = pin_desc[i].name;
>  		group->pin = pin_desc[i].number;
> -- 
> 2.34.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] pinctrl: check memory returned by devm_kasprintf()

[Andy Shevchenko]

On Thu, Jun 15, 2023 at 01:53:30PM +0300, Claudiu Beznea wrote:
> Hi,
> 
> While browsing some code I noticed that there are places where pointer
> returned by devm_kasprintf() or kasprintf() is not checked. Thus I've
> tooked the chance and fixed this (by updating kmerr.cocci script,
> changes published at [1]).

Thank you for fixing this!
Individual patches are commented by me.

> [1] https://lore.kernel.org/all/20230530074044.1603426-1-claudiu.beznea@microchip.com/
> 
> Claudiu Beznea (3):
>   pinctrl: mcp23s08: check return value of {devm_}kasprintf()
>   pinctrl: microchip-sgpio: check return value of devm_kasprintf()
>   pinctrl: at91-pio4: check return value of devm_kasprintf()
> 
>  drivers/pinctrl/pinctrl-at91-pio4.c       | 2 ++
>  drivers/pinctrl/pinctrl-mcp23s08_spi.c    | 3 +++
>  drivers/pinctrl/pinctrl-microchip-sgpio.c | 3 +++
>  3 files changed, 8 insertions(+)
> 
> -- 
> 2.34.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/3] pinctrl: check memory returned by devm_kasprintf()

[Linus Walleij]

Hi Claudiu,

thanks for your patches!

On Thu, Jun 15, 2023 at 12:53 PM Claudiu Beznea
<claudiu.beznea@microchip.com> wrote:

> While browsing some code I noticed that there are places where pointer
> returned by devm_kasprintf() or kasprintf() is not checked. Thus I've
> tooked the chance and fixed this (by updating kmerr.cocci script,
> changes published at [1]).

I applied patch 2/3 and 3/3 you only need to resent the first patch
after looking into the comments from Andy.

Yours,
Linus Walleij