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@xxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxx>
> ---
>  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