[PATCH] clk: divider: Fix overflow in clk_divider_bestdiv() for large rate requests

[PATCH] clk: divider: Fix overflow in clk_divider_bestdiv() for large rate requests

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

clk_divider_bestdiv() clamps maxdiv using:

    maxdiv = min(ULONG_MAX / rate, maxdiv);

to avoid overflow in rate * i. However requests like
clk_round_rate(clk, ULONG_MAX), which are used to determine the maximum
supported rate of a clock, result in maxdiv being clamped to 1. If no
valid divider of 1 exists in the table the loop is never entered and
bestdiv falls back to the maximum divider with the minimum parent rate,
causing clk_round_rate(clk, ULONG_MAX) to incorrectly return the minimum
supported rate instead of the maximum.

Fix this by replacing the maxdiv clamping and the unprotected rate * i
multiplications with check_mul_overflow(), clamping target_parent_rate
to ULONG_MAX on overflow. This allows the loop to iterate all valid
dividers regardless of the requested rate, and clk_hw_round_rate() with
ULONG_MAX will correctly return the maximum supported parent rate.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/clk/clk-divider.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 45e7ebde4a8b..dc486c2aa946 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/log2.h>
+#include <linux/overflow.h>
 
 /*
  * DOC: basic adjustable divider clock that cannot gate
@@ -301,6 +302,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 	int i, bestdiv = 0;
 	unsigned long parent_rate, best = 0, now, maxdiv;
 	unsigned long parent_rate_saved = *best_parent_rate;
+	unsigned long target_parent_rate;
 
 	if (!rate)
 		rate = 1;
@@ -315,15 +317,11 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 		return bestdiv;
 	}
 
-	/*
-	 * The maximum divider we can use without overflowing
-	 * unsigned long in rate * i below
-	 */
-	maxdiv = min(ULONG_MAX / rate, maxdiv);
-
 	for (i = _next_div(table, 0, flags); i <= maxdiv;
 					     i = _next_div(table, i, flags)) {
-		if (rate * i == parent_rate_saved) {
+		if (check_mul_overflow(rate, (unsigned long)i, &target_parent_rate))
+			target_parent_rate = ULONG_MAX;
+		if (target_parent_rate == parent_rate_saved) {
 			/*
 			 * It's the most ideal case if the requested rate can be
 			 * divided from parent clock without needing to change
@@ -332,7 +330,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
 			*best_parent_rate = parent_rate_saved;
 			return i;
 		}
-		parent_rate = clk_hw_round_rate(parent, rate * i);
+		parent_rate = clk_hw_round_rate(parent, target_parent_rate);
 		now = DIV_ROUND_UP_ULL((u64)parent_rate, i);
 		if (_is_best_div(rate, now, best, flags)) {
 			bestdiv = i;
-- 
2.53.0

Re: [PATCH] clk: divider: Fix overflow in clk_divider_bestdiv() for large rate requests

[Stephen Boyd]

Quoting Prabhakar (2026-04-08 02:48:14)
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> clk_divider_bestdiv() clamps maxdiv using:
> 
>     maxdiv = min(ULONG_MAX / rate, maxdiv);
> 
> to avoid overflow in rate * i. However requests like
> clk_round_rate(clk, ULONG_MAX), which are used to determine the maximum
> supported rate of a clock, result in maxdiv being clamped to 1. If no
> valid divider of 1 exists in the table the loop is never entered and
> bestdiv falls back to the maximum divider with the minimum parent rate,
> causing clk_round_rate(clk, ULONG_MAX) to incorrectly return the minimum
> supported rate instead of the maximum.
> 
> Fix this by replacing the maxdiv clamping and the unprotected rate * i
> multiplications with check_mul_overflow(), clamping target_parent_rate
> to ULONG_MAX on overflow. This allows the loop to iterate all valid
> dividers regardless of the requested rate, and clk_hw_round_rate() with
> ULONG_MAX will correctly return the maximum supported parent rate.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/clk/clk-divider.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

Please add kunit tests to show the broken behavior that you're fixing.
Make a clk-divider_test.c file for this instead of adding it to the
clk_test.c file.

> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 45e7ebde4a8b..dc486c2aa946 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -15,6 +15,7 @@
>  #include <linux/err.h>
>  #include <linux/string.h>
>  #include <linux/log2.h>
> +#include <linux/overflow.h>
>  
>  /*
>   * DOC: basic adjustable divider clock that cannot gate
> @@ -301,6 +302,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
>         int i, bestdiv = 0;
>         unsigned long parent_rate, best = 0, now, maxdiv;
>         unsigned long parent_rate_saved = *best_parent_rate;
> +       unsigned long target_parent_rate;
>  
>         if (!rate)
>                 rate = 1;
> @@ -315,15 +317,11 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
>                 return bestdiv;
>         }
>  
> -       /*
> -        * The maximum divider we can use without overflowing
> -        * unsigned long in rate * i below
> -        */
> -       maxdiv = min(ULONG_MAX / rate, maxdiv);
> -
>         for (i = _next_div(table, 0, flags); i <= maxdiv;
>                                              i = _next_div(table, i, flags)) {
> -               if (rate * i == parent_rate_saved) {
> +               if (check_mul_overflow(rate, (unsigned long)i, &target_parent_rate))

Please add some sort of comment above this if condition to tell us what
a target_parent_rate of ULONG_MAX means and why overflowing we set the
rate to that.

> +                       target_parent_rate = ULONG_MAX;
> +               if (target_parent_rate == parent_rate_saved) {
>                         /*
>                          * It's the most ideal case if the requested rate can be
>                          * divided from parent clock without needing to change

Re: [PATCH] clk: divider: Fix overflow in clk_divider_bestdiv() for large rate requests

[Lad, Prabhakar]

Hi Stephen,

Thank you for the review.

On Sun, Apr 12, 2026 at 1:50 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Prabhakar (2026-04-08 02:48:14)
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > clk_divider_bestdiv() clamps maxdiv using:
> >
> >     maxdiv = min(ULONG_MAX / rate, maxdiv);
> >
> > to avoid overflow in rate * i. However requests like
> > clk_round_rate(clk, ULONG_MAX), which are used to determine the maximum
> > supported rate of a clock, result in maxdiv being clamped to 1. If no
> > valid divider of 1 exists in the table the loop is never entered and
> > bestdiv falls back to the maximum divider with the minimum parent rate,
> > causing clk_round_rate(clk, ULONG_MAX) to incorrectly return the minimum
> > supported rate instead of the maximum.
> >
> > Fix this by replacing the maxdiv clamping and the unprotected rate * i
> > multiplications with check_mul_overflow(), clamping target_parent_rate
> > to ULONG_MAX on overflow. This allows the loop to iterate all valid
> > dividers regardless of the requested rate, and clk_hw_round_rate() with
> > ULONG_MAX will correctly return the maximum supported parent rate.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/clk/clk-divider.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
>
> Please add kunit tests to show the broken behavior that you're fixing.
> Make a clk-divider_test.c file for this instead of adding it to the
> clk_test.c file.
>
OK.

> >
> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > index 45e7ebde4a8b..dc486c2aa946 100644
> > --- a/drivers/clk/clk-divider.c
> > +++ b/drivers/clk/clk-divider.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/err.h>
> >  #include <linux/string.h>
> >  #include <linux/log2.h>
> > +#include <linux/overflow.h>
> >
> >  /*
> >   * DOC: basic adjustable divider clock that cannot gate
> > @@ -301,6 +302,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
> >         int i, bestdiv = 0;
> >         unsigned long parent_rate, best = 0, now, maxdiv;
> >         unsigned long parent_rate_saved = *best_parent_rate;
> > +       unsigned long target_parent_rate;
> >
> >         if (!rate)
> >                 rate = 1;
> > @@ -315,15 +317,11 @@ static int clk_divider_bestdiv(struct clk_hw *hw, struct clk_hw *parent,
> >                 return bestdiv;
> >         }
> >
> > -       /*
> > -        * The maximum divider we can use without overflowing
> > -        * unsigned long in rate * i below
> > -        */
> > -       maxdiv = min(ULONG_MAX / rate, maxdiv);
> > -
> >         for (i = _next_div(table, 0, flags); i <= maxdiv;
> >                                              i = _next_div(table, i, flags)) {
> > -               if (rate * i == parent_rate_saved) {
> > +               if (check_mul_overflow(rate, (unsigned long)i, &target_parent_rate))
>
> Please add some sort of comment above this if condition to tell us what
> a target_parent_rate of ULONG_MAX means and why overflowing we set the
> rate to that.
>
Ok, I will add a comment for and send a v2.

Cheers,
Prabhakar

> > +                       target_parent_rate = ULONG_MAX;
> > +               if (target_parent_rate == parent_rate_saved) {
> >                         /*
> >                          * It's the most ideal case if the requested rate can be
> >                          * divided from parent clock without needing to change