drivers/clk/davinci/psc.c | 5 +++++ 1 file changed, 5 insertions(+)
devm_kasprintf() returns NULL when memory allocation fails. Currently,
davinci_lpsc_clk_register() does not check for this case, which results
in a NULL pointer dereference.
Add NULL check after devm_kasprintf() to prevent this issue and ensuring
no resources are left allocated.
Fixes: c6ed4d734bc7 ("clk: davinci: New driver for davinci PSC clocks")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
drivers/clk/davinci/psc.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
index b48322176c21..f3ee9397bb0c 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -277,6 +277,11 @@ davinci_lpsc_clk_register(struct device *dev, const char *name,
lpsc->pm_domain.name = devm_kasprintf(dev, GFP_KERNEL, "%s: %s",
best_dev_name(dev), name);
+ if (!lpsc->pm_domain.name) {
+ clk_hw_unregister(&lpsc->hw);
+ kfree(lpsc);
+ return ERR_PTR(-ENOMEM);
+ }
lpsc->pm_domain.attach_dev = davinci_psc_genpd_attach_dev;
lpsc->pm_domain.detach_dev = davinci_psc_genpd_detach_dev;
lpsc->pm_domain.flags = GENPD_FLAG_PM_CLK;
--
2.34.1Quoting Henry Martin (2025-04-01 06:13:41)
> devm_kasprintf() returns NULL when memory allocation fails. Currently,
> davinci_lpsc_clk_register() does not check for this case, which results
> in a NULL pointer dereference.
>
> Add NULL check after devm_kasprintf() to prevent this issue and ensuring
> no resources are left allocated.
>
> Fixes: c6ed4d734bc7 ("clk: davinci: New driver for davinci PSC clocks")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
Applied to clk-next
Hi Michael, Stephen,
I wanted to follow up on my previous patch submission to check if there are any
additional feedback or changes you'd like me to address. If so, I’d be happy to
incorporate them and send a v2.
Regards,
Henry
Henry Martin <bsdhenrymartin@gmail.com> 于2025年4月1日周二 21:13写道：
>
> devm_kasprintf() returns NULL when memory allocation fails. Currently,
> davinci_lpsc_clk_register() does not check for this case, which results
> in a NULL pointer dereference.
>
> Add NULL check after devm_kasprintf() to prevent this issue and ensuring
> no resources are left allocated.
>
> Fixes: c6ed4d734bc7 ("clk: davinci: New driver for davinci PSC clocks")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
> drivers/clk/davinci/psc.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
> index b48322176c21..f3ee9397bb0c 100644
> --- a/drivers/clk/davinci/psc.c
> +++ b/drivers/clk/davinci/psc.c
> @@ -277,6 +277,11 @@ davinci_lpsc_clk_register(struct device *dev, const char *name,
>
> lpsc->pm_domain.name = devm_kasprintf(dev, GFP_KERNEL, "%s: %s",
> best_dev_name(dev), name);
> + if (!lpsc->pm_domain.name) {
> + clk_hw_unregister(&lpsc->hw);
> + kfree(lpsc);
> + return ERR_PTR(-ENOMEM);
> + }
> lpsc->pm_domain.attach_dev = davinci_psc_genpd_attach_dev;
> lpsc->pm_domain.detach_dev = davinci_psc_genpd_detach_dev;
> lpsc->pm_domain.flags = GENPD_FLAG_PM_CLK;
> --
> 2.34.1
>
> devm_kasprintf() return NULL if memory allocation fails. Currently,
…
call? failed?
> Add NULL check after devm_kasprintf() to prevent this issue.
I propose to avoid duplicate source code also for the completion of
the corresponding exception handling.
* You may avoid repeated function calls by using another label instead.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution(copy_process()fromLinuxkernel)
* How do you think about to benefit any more from the application of the attribute “__free”?
https://elixir.bootlin.com/linux/v6.14-rc6/source/include/linux/slab.h#L472
Regards,
Markus
On 4/1/25 11:46 AM, Markus Elfring wrote: >> devm_kasprintf() return NULL if memory allocation fails. Currently, > … > call? failed? > > >> Add NULL check after devm_kasprintf() to prevent this issue. > > I propose to avoid duplicate source code also for the completion of > the corresponding exception handling. > > * You may avoid repeated function calls by using another label instead. > https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution(copy_process()fromLinuxkernel) That would be OK too. I didn't worry about it in this case though since we are only duplicating 1 very short line of code. And the smaller diff has a better chance of successfully backporting to older stable kernels that will also pick up this patch. > > * How do you think about to benefit any more from the application of the attribute “__free”? > https://elixir.bootlin.com/linux/v6.14-rc6/source/include/linux/slab.h#L472 Not a good fit for this specific use case. > > > Regards, > Markus
>> I propose to avoid duplicate source code also for the completion of >> the corresponding exception handling. >> >> * You may avoid repeated function calls by using another label instead. >> https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution(copy_process()fromLinuxkernel) > > That would be OK too. Thanks for another bit of positive feedback. > I didn't worry about it in this case though > since we are only duplicating 1 very short line of code. And the > smaller diff has a better chance of successfully backporting to older > stable kernels that will also pick up this patch. Such development concerns can be clarified further. >> * How do you think about to benefit any more from the application of the attribute “__free”? >> https://elixir.bootlin.com/linux/v6.14-rc6/source/include/linux/slab.h#L472 > > Not a good fit for this specific use case. The acceptance might be growing for such a software design option. Would you like to clarify any more why the function “kzalloc” is still called here (instead of the variant “devm_kzalloc”)? https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/clk/davinci/psc.c#L242 Regards, Markus
On 4/2/25 2:48 AM, Markus Elfring wrote: > > Would you like to clarify any more why the function “kzalloc” is still called here > (instead of the variant “devm_kzalloc”)? > https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/clk/davinci/psc.c#L242 It is a case of "if it isn't broke, don't fix it". While there is room for some small improvements like that, it does come at a cost of time and energy to make those improvements. > > Regards, > Markus
On 4/1/25 8:13 AM, Henry Martin wrote:
> devm_kasprintf() returns NULL when memory allocation fails. Currently,
> davinci_lpsc_clk_register() does not check for this case, which results
> in a NULL pointer dereference.
>
> Add NULL check after devm_kasprintf() to prevent this issue and ensuring
> no resources are left allocated.
>
> Fixes: c6ed4d734bc7 ("clk: davinci: New driver for davinci PSC clocks")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
Reviewed-by: David Lechner <david@lechnology.com>
© 2016 - 2025 Red Hat, Inc.