clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

[PATCH] clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

Posted by Markus Elfring 3 months, 1 week ago

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 31 Oct 2025 15:20:22 +0100

* Reuse existing functionality from devm_kmemdup_array()
  instead of keeping duplicate source code.

* Prevent a null pointer dereference.


The source code was transformed by using the Coccinelle software.

Fixes: 3c13933c60338ce6fb2369bd0e93f91e52ddc17d ("clk: keystone: sci-clk: add support for dynamically probing clocks")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/keystone/sci-clk.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
index a4b42811de55..0c93c83d4776 100644
--- a/drivers/clk/keystone/sci-clk.c
+++ b/drivers/clk/keystone/sci-clk.c
@@ -456,12 +456,14 @@ static int ti_sci_scan_clocks_from_fw(struct sci_clk_provider *provider)
 		gap_size = 0;
 
 		if (num_clks == max_clks) {
-			tmp_clks = devm_kmalloc_array(dev, max_clks + 64,
-						      sizeof(sci_clk),
-						      GFP_KERNEL);
-			memcpy(tmp_clks, clks, max_clks * sizeof(sci_clk));
+			tmp_clks = devm_kmemdup_array(dev, clks, max_clks + 64,
+						      sizeof(sci_clk), GFP_KERNEL);
 			if (max_clks)
 				devm_kfree(dev, clks);
+
+			if (!tmp_clks)
+				return -ENOMEM;
+
 			max_clks += 64;
 			clks = tmp_clks;
 		}
-- 
2.51.1

Re: [PATCH] clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

Posted by Andy Shevchenko 3 months, 1 week ago

On Fri, Oct 31, 2025 at 03:33:24PM +0100, Markus Elfring wrote:
> 
> * Reuse existing functionality from devm_kmemdup_array()
>   instead of keeping duplicate source code.
> 
> * Prevent a null pointer dereference.
> 
> 
> The source code was transformed by using the Coccinelle software.
> 
> Fixes: 3c13933c60338ce6fb2369bd0e93f91e52ddc17d ("clk: keystone: sci-clk: add support for dynamically probing clocks")

Wrong format of Fixes tag, please keep the SHA at bare minumum.

...

>  		if (num_clks == max_clks) {
> -			tmp_clks = devm_kmalloc_array(dev, max_clks + 64,
> -						      sizeof(sci_clk),
> -						      GFP_KERNEL);
> -			memcpy(tmp_clks, clks, max_clks * sizeof(sci_clk));
> +			tmp_clks = devm_kmemdup_array(dev, clks, max_clks + 64,
> +						      sizeof(sci_clk), GFP_KERNEL);
>  			if (max_clks)
>  				devm_kfree(dev, clks);

> +			if (!tmp_clks)
> +				return -ENOMEM;

You haven't read the code, right?
This now will work differently. Would it be acceptable?

>  			max_clks += 64;
>  			clks = tmp_clks;
>  		}

-- 
With Best Regards,
Andy Shevchenko

Re: clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

Posted by Markus Elfring 3 months, 1 week ago

>> Fixes: 3c13933c60338ce6fb2369bd0e93f91e52ddc17d ("clk: keystone: sci-clk: add support for dynamically probing clocks")
> 
> Wrong format of Fixes tag, please keep the SHA at bare minumum.

I got an other impression.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc3#n145

Would you find information from the commit 6356f18f09dc0781650c4f128ea48745fa48c415
("Align git commit ID abbreviation guidelines and checks") interesting also
for this technical detail?

Regards,
Markus

Re: clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

Posted by Andy Shevchenko 3 months ago

On Fri, Oct 31, 2025 at 04:06:59PM +0100, Markus Elfring wrote:
> >> Fixes: 3c13933c60338ce6fb2369bd0e93f91e52ddc17d ("clk: keystone: sci-clk: add support for dynamically probing clocks")
> > 
> > Wrong format of Fixes tag, please keep the SHA at bare minumum.
> 
> I got an other impression.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc3#n145
> 
> Would you find information from the commit 6356f18f09dc0781650c4f128ea48745fa48c415
> ("Align git commit ID abbreviation guidelines and checks") interesting also
> for this technical detail?

Even the above mentioned commit doesn't do like you have done. 12+ doesn't mean
the full length. The rule of thumb is to use the minimum that doesn't give a
clash. And you may read the Linus' thoughts on the Geert's work. I.o.w. don't solve
the issue that doesn't exist.

-- 
With Best Regards,
Andy Shevchenko

Re: clk: keystone: sci-clk: use devm_kmemdup_array() once more in ti_sci_scan_clocks_from_fw()

Posted by Markus Elfring 3 months ago

>>>> Fixes: 3c13933c60338ce6fb2369bd0e93f91e52ddc17d ("clk: keystone: sci-clk: add support for dynamically probing clocks")

Will further adjustments become more helpful for the affected function implementation?

Regards,
Markus