| Message ID | 20240606161028.2986587-3-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: Switch to use kmemdup_array() | expand |
Hi Andy, Am Donnerstag, 6. Juni 2024, 18:09:32 CEST schrieb Andy Shevchenko: > Let the kememdup_array() take care about multiplication and possible > overflows. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/clk/rockchip/clk-cpu.c | 5 ++--- > drivers/clk/rockchip/clk-pll.c | 8 ++++---- > 2 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c > index 6ea7fba9f9e5..398a226ad34e 100644 > --- a/drivers/clk/rockchip/clk-cpu.c > +++ b/drivers/clk/rockchip/clk-cpu.c > @@ -369,9 +369,8 @@ struct clk *rockchip_clk_register_cpuclk(const char *name, > > if (nrates > 0) { > cpuclk->rate_count = nrates; > - cpuclk->rate_table = kmemdup(rates, > - sizeof(*rates) * nrates, > - GFP_KERNEL); > + cpuclk->rate_table = kmemdup_array(rates, nrates, sizeof(*rates), > + GFP_KERNEL); are you sure the param order is correct? According to [0], it's (src, element_size, count, gfp), while above (and below) element_size and count seems switched in the kmemdup_array calls. Heiko [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/util.c#n149 > if (!cpuclk->rate_table) { > ret = -ENOMEM; > goto unregister_notifier; > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > index 2d42eb628926..606ce5458f54 100644 > --- a/drivers/clk/rockchip/clk-pll.c > +++ b/drivers/clk/rockchip/clk-pll.c > @@ -1136,10 +1136,10 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, > len++; > > pll->rate_count = len; > - pll->rate_table = kmemdup(rate_table, > - pll->rate_count * > - sizeof(struct rockchip_pll_rate_table), > - GFP_KERNEL); > + pll->rate_table = kmemdup_array(rate_table, > + pll->rate_count, > + sizeof(*pll->rate_table), > + GFP_KERNEL); > WARN(!pll->rate_table, > "%s: could not allocate rate table for %s\n", > __func__, name); >
On Fri, Jun 07, 2024 at 10:13:04AM +0200, Heiko Stübner wrote: > Am Donnerstag, 6. Juni 2024, 18:09:32 CEST schrieb Andy Shevchenko: ... > > - cpuclk->rate_table = kmemdup(rates, > > - sizeof(*rates) * nrates, > > - GFP_KERNEL); > > + cpuclk->rate_table = kmemdup_array(rates, nrates, sizeof(*rates), > > + GFP_KERNEL); > > are you sure the param order is correct? > > According to [0], it's (src, element_size, count, gfp), while above > (and below) element_size and count seems switched in the > kmemdup_array calls. I'm glad you asked. The parameter order is going to be fixed [1]. > [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/util.c#n149 [1]: 0ee14725471c ("mm/util: Swap kmemdup_array() arguments")
Am Dienstag, 11. Juni 2024, 15:20:05 CEST schrieb Andy Shevchenko: > On Fri, Jun 07, 2024 at 10:13:04AM +0200, Heiko Stübner wrote: > > Am Donnerstag, 6. Juni 2024, 18:09:32 CEST schrieb Andy Shevchenko: > > ... > > > > - cpuclk->rate_table = kmemdup(rates, > > > - sizeof(*rates) * nrates, > > > - GFP_KERNEL); > > > + cpuclk->rate_table = kmemdup_array(rates, nrates, sizeof(*rates), > > > + GFP_KERNEL); > > > > are you sure the param order is correct? > > > > According to [0], it's (src, element_size, count, gfp), while above > > (and below) element_size and count seems switched in the > > kmemdup_array calls. > > I'm glad you asked. The parameter order is going to be fixed [1]. > > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/util.c#n149 > > [1]: 0ee14725471c ("mm/util: Swap kmemdup_array() arguments") ah that clears it up :-) Thanks for the pointer Heiko
diff --git a/drivers/clk/rockchip/clk-cpu.c b/drivers/clk/rockchip/clk-cpu.c index 6ea7fba9f9e5..398a226ad34e 100644 --- a/drivers/clk/rockchip/clk-cpu.c +++ b/drivers/clk/rockchip/clk-cpu.c @@ -369,9 +369,8 @@ struct clk *rockchip_clk_register_cpuclk(const char *name, if (nrates > 0) { cpuclk->rate_count = nrates; - cpuclk->rate_table = kmemdup(rates, - sizeof(*rates) * nrates, - GFP_KERNEL); + cpuclk->rate_table = kmemdup_array(rates, nrates, sizeof(*rates), + GFP_KERNEL); if (!cpuclk->rate_table) { ret = -ENOMEM; goto unregister_notifier; diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c index 2d42eb628926..606ce5458f54 100644 --- a/drivers/clk/rockchip/clk-pll.c +++ b/drivers/clk/rockchip/clk-pll.c @@ -1136,10 +1136,10 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, len++; pll->rate_count = len; - pll->rate_table = kmemdup(rate_table, - pll->rate_count * - sizeof(struct rockchip_pll_rate_table), - GFP_KERNEL); + pll->rate_table = kmemdup_array(rate_table, + pll->rate_count, + sizeof(*pll->rate_table), + GFP_KERNEL); WARN(!pll->rate_table, "%s: could not allocate rate table for %s\n", __func__, name);
Let the kememdup_array() take care about multiplication and possible overflows. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/clk/rockchip/clk-cpu.c | 5 ++--- drivers/clk/rockchip/clk-pll.c | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-)