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]). Along with it some other places where
resources may need to be freed on failure paths were updated.
Thank you,
Claudiu Beznea
[1] https://lore.kernel.org/all/[email protected]/
Claudiu Beznea (8):
clk: vc5: check memory returned by kasprintf()
clk: cdce925: check return value of kasprintf()
clk: si5341: return error if one synth clock registration fails
clk: si5341: check return value of {devm_}kasprintf()
clk: si5341: free unused memory on probe failure
clk: keystone: sci-clk: check return value of kasprintf()
clk: ti: clkctrl: check return value of kasprintf()
clk: clocking-wizard: check return value of devm_kasprintf()
drivers/clk/clk-cdce925.c | 12 +++++++
drivers/clk/clk-si5341.c | 38 +++++++++++++---------
drivers/clk/clk-versaclock5.c | 29 +++++++++++++++++
drivers/clk/keystone/sci-clk.c | 2 ++
drivers/clk/ti/clkctrl.c | 7 ++++
drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 5 +++
6 files changed, 78 insertions(+), 15 deletions(-)
--
2.34.1
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: 2046338dcbc6 ("ARM: mxs: Use soc bus infrastructure")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/xilinx/clk-xlnx-clock-wizard.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
index e83f104fad02..20e0e91552bc 100644
--- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
+++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
@@ -648,6 +648,11 @@ static int clk_wzrd_probe(struct platform_device *pdev)
}
clkout_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_out0", dev_name(&pdev->dev));
+ if (!clkout_name) {
+ ret = -ENOMEM;
+ goto err_disable_clk;
+ }
+
if (nr_outputs == 1) {
clk_wzrd->clkout[0] = clk_wzrd_register_divider
(&pdev->dev, clkout_name,
--
2.34.1
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: 19fbbbbcd3a3 ("Add TI CDCE925 I2C controlled clock synthesizer driver")
Depends-on: e665f029a283 ("clk: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/clk-cdce925.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/clk/clk-cdce925.c b/drivers/clk/clk-cdce925.c
index 6350682f7e6d..87890669297d 100644
--- a/drivers/clk/clk-cdce925.c
+++ b/drivers/clk/clk-cdce925.c
@@ -701,6 +701,10 @@ static int cdce925_probe(struct i2c_client *client)
for (i = 0; i < data->chip_info->num_plls; ++i) {
pll_clk_name[i] = kasprintf(GFP_KERNEL, "%pOFn.pll%d",
client->dev.of_node, i);
+ if (!pll_clk_name[i]) {
+ err = -ENOMEM;
+ goto error;
+ }
init.name = pll_clk_name[i];
data->pll[i].chip = data;
data->pll[i].hw.init = &init;
@@ -742,6 +746,10 @@ static int cdce925_probe(struct i2c_client *client)
init.num_parents = 1;
init.parent_names = &parent_name; /* Mux Y1 to input */
init.name = kasprintf(GFP_KERNEL, "%pOFn.Y1", client->dev.of_node);
+ if (!init.name) {
+ err = -ENOMEM;
+ goto error;
+ }
data->clk[0].chip = data;
data->clk[0].hw.init = &init;
data->clk[0].index = 0;
@@ -760,6 +768,10 @@ static int cdce925_probe(struct i2c_client *client)
for (i = 1; i < data->chip_info->num_outputs; ++i) {
init.name = kasprintf(GFP_KERNEL, "%pOFn.Y%d",
client->dev.of_node, i+1);
+ if (!init.name) {
+ err = -ENOMEM;
+ goto error;
+ }
data->clk[i].chip = data;
data->clk[i].hw.init = &init;
data->clk[i].index = i;
--
2.34.1
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: 852049594b9a ("clk: ti: clkctrl: convert subclocks to use proper names also")
Fixes: 6c3090520554 ("clk: ti: clkctrl: Fix hidden dependency to node name")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/ti/clkctrl.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index b6fce916967c..8c40f10280b7 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -258,6 +258,9 @@ static const char * __init clkctrl_get_clock_name(struct device_node *np,
if (clkctrl_name && !legacy_naming) {
clock_name = kasprintf(GFP_KERNEL, "%s-clkctrl:%04x:%d",
clkctrl_name, offset, index);
+ if (!clock_name)
+ return NULL;
+
strreplace(clock_name, '_', '-');
return clock_name;
@@ -586,6 +589,10 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
if (clkctrl_name) {
provider->clkdm_name = kasprintf(GFP_KERNEL,
"%s_clkdm", clkctrl_name);
+ if (!provider->clkdm_name) {
+ kfree(provider);
+ return;
+ }
goto clkdm_found;
}
--
2.34.1
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: f491276a5168 ("clk: vc5: Allow Versaclock driver to support multiple instances")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/clk-versaclock5.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
index fa71a57875ce..40fdf2564aa7 100644
--- a/drivers/clk/clk-versaclock5.c
+++ b/drivers/clk/clk-versaclock5.c
@@ -1028,6 +1028,11 @@ static int vc5_probe(struct i2c_client *client)
}
init.name = kasprintf(GFP_KERNEL, "%pOFn.mux", client->dev.of_node);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
+
init.ops = &vc5_mux_ops;
init.flags = 0;
init.parent_names = parent_names;
@@ -1042,6 +1047,10 @@ static int vc5_probe(struct i2c_client *client)
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.dbl",
client->dev.of_node);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_dbl_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
@@ -1057,6 +1066,10 @@ static int vc5_probe(struct i2c_client *client)
/* Register PFD */
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.pfd", client->dev.of_node);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_pfd_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
@@ -1074,6 +1087,10 @@ static int vc5_probe(struct i2c_client *client)
/* Register PLL */
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.pll", client->dev.of_node);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_pll_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
@@ -1093,6 +1110,10 @@ static int vc5_probe(struct i2c_client *client)
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.fod%d",
client->dev.of_node, idx);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_fod_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
@@ -1111,6 +1132,10 @@ static int vc5_probe(struct i2c_client *client)
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.out0_sel_i2cb",
client->dev.of_node);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_clk_out_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
@@ -1137,6 +1162,10 @@ static int vc5_probe(struct i2c_client *client)
memset(&init, 0, sizeof(init));
init.name = kasprintf(GFP_KERNEL, "%pOFn.out%d",
client->dev.of_node, idx + 1);
+ if (!init.name) {
+ ret = -ENOMEM;
+ goto err_clk;
+ }
init.ops = &vc5_clk_out_ops;
init.flags = CLK_SET_RATE_PARENT;
init.parent_names = parent_names;
--
2.34.1
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: b745c0794e2f ("clk: keystone: Add sci-clk driver support")
Depends-on: 96488c09b0f4 ("clk: keystone: sci-clk: cut down the clock name length")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/keystone/sci-clk.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index 910ecd58c4ca..6c1df4f11536 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -294,6 +294,8 @@ static int _sci_clk_build(struct sci_clk_provider *provider,
name = kasprintf(GFP_KERNEL, "clk:%d:%d", sci_clk->dev_id,
sci_clk->clk_id);
+ if (!name)
+ return -ENOMEM;
init.name = name;
--
2.34.1
{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: 3044a860fd09 ("clk: Add Si5341/Si5340 driver")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/clk-si5341.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index 6dca3288c894..b2cf7edc8b30 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -1697,6 +1697,10 @@ static int si5341_probe(struct i2c_client *client)
for (i = 0; i < data->num_synth; ++i) {
synth_clock_names[i] = devm_kasprintf(&client->dev, GFP_KERNEL,
"%s.N%u", client->dev.of_node->name, i);
+ if (!synth_clock_names[i]) {
+ err = -ENOMEM;
+ goto free_clk_names;
+ }
init.name = synth_clock_names[i];
data->synth[i].index = i;
data->synth[i].data = data;
@@ -1715,6 +1719,10 @@ static int si5341_probe(struct i2c_client *client)
for (i = 0; i < data->num_outputs; ++i) {
init.name = kasprintf(GFP_KERNEL, "%s.%d",
client->dev.of_node->name, i);
+ if (!init.name) {
+ err = -ENOMEM;
+ goto free_clk_names;
+ }
init.flags = config[i].synth_master ? CLK_SET_RATE_PARENT : 0;
data->clk[i].index = i;
data->clk[i].data = data;
--
2.34.1
Pointers from synth_clock_names[] should be freed at the end of probe
either on probe success or failure path.
Fixes: b7bbf6ec4940 ("clk: si5341: Allow different output VDD_SEL values")
Fixes: 9b13ff4340df ("clk: si5341: Add sysfs properties to allow checking/resetting device faults")
Signed-off-by: Claudiu Beznea <[email protected]>
---
drivers/clk/clk-si5341.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk-si5341.c b/drivers/clk/clk-si5341.c
index b2cf7edc8b30..c7d8cbd22bac 100644
--- a/drivers/clk/clk-si5341.c
+++ b/drivers/clk/clk-si5341.c
@@ -1744,7 +1744,7 @@ static int si5341_probe(struct i2c_client *client)
if (err) {
dev_err(&client->dev,
"output %u registration failed\n", i);
- goto cleanup;
+ goto free_clk_names;
}
if (config[i].always_on)
clk_prepare(data->clk[i].hw.clk);
@@ -1754,7 +1754,7 @@ static int si5341_probe(struct i2c_client *client)
data);
if (err) {
dev_err(&client->dev, "unable to add clk provider\n");
- goto cleanup;
+ goto free_clk_names;
}
if (initialization_required) {
@@ -1762,11 +1762,11 @@ static int si5341_probe(struct i2c_client *client)
regcache_cache_only(data->regmap, false);
err = regcache_sync(data->regmap);
if (err < 0)
- goto cleanup;
+ goto free_clk_names;
err = si5341_finalize_defaults(data);
if (err < 0)
- goto cleanup;
+ goto free_clk_names;
}
/* wait for device to report input clock present and PLL lock */
@@ -1775,21 +1775,19 @@ static int si5341_probe(struct i2c_client *client)
10000, 250000);
if (err) {
dev_err(&client->dev, "Error waiting for input clock or PLL lock\n");
- goto cleanup;
+ goto free_clk_names;
}
/* clear sticky alarm bits from initialization */
err = regmap_write(data->regmap, SI5341_STATUS_STICKY, 0);
if (err) {
dev_err(&client->dev, "unable to clear sticky status\n");
- goto cleanup;
+ goto free_clk_names;
}
err = sysfs_create_files(&client->dev.kobj, si5341_attributes);
- if (err) {
+ if (err)
dev_err(&client->dev, "unable to create sysfs files\n");
- goto cleanup;
- }
free_clk_names:
/* Free the names, clk framework makes copies */
--
2.34.1
* Claudiu Beznea <[email protected]> [230530 09:41]:
> 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: Tony Lindgren <[email protected]>
* Claudiu Beznea <[email protected]> [230530 09:41]:
> 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: Tony Lindgren <[email protected]>
Hi Claudiu,
On Tue, 30 May 2023 12:39:06 +0300
Claudiu Beznea <[email protected]> wrote:
> 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: f491276a5168 ("clk: vc5: Allow Versaclock driver to support multiple instances")
> Signed-off-by: Claudiu Beznea <[email protected]>
I think this driver would benefit a bit of refactoring, moving all
those similar sections to a single function.
But for the time being your change is fine:
Reviewed-by: Luca Ceresoli <[email protected]>
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Quoting Claudiu Beznea (2023-05-30 02:39:07)
> 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: 19fbbbbcd3a3 ("Add TI CDCE925 I2C controlled clock synthesizer driver")
> Depends-on: e665f029a283 ("clk: Convert to using %pOFn instead of device_node.name")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:12)
> 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: 852049594b9a ("clk: ti: clkctrl: convert subclocks to use proper names also")
> Fixes: 6c3090520554 ("clk: ti: clkctrl: Fix hidden dependency to node name")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:11)
> 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: b745c0794e2f ("clk: keystone: Add sci-clk driver support")
> Depends-on: 96488c09b0f4 ("clk: keystone: sci-clk: cut down the clock name length")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:13)
> 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: 2046338dcbc6 ("ARM: mxs: Use soc bus infrastructure")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:09)
> {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: 3044a860fd09 ("clk: Add Si5341/Si5340 driver")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:06)
> 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: f491276a5168 ("clk: vc5: Allow Versaclock driver to support multiple instances")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next
Quoting Claudiu Beznea (2023-05-30 02:39:10)
> Pointers from synth_clock_names[] should be freed at the end of probe
> either on probe success or failure path.
>
> Fixes: b7bbf6ec4940 ("clk: si5341: Allow different output VDD_SEL values")
> Fixes: 9b13ff4340df ("clk: si5341: Add sysfs properties to allow checking/resetting device faults")
> Signed-off-by: Claudiu Beznea <[email protected]>
> ---
Applied to clk-next