Re: [PATCH] clk: sunxi: add explicit casting to prevent overflow

[David Laight]

On Mon, 20 Jan 2025 11:47:16 +0300
Anastasia Belova <abelova@xxxxxxxxxxxxx> wrote:

> If n = 255, the result of multiplication of n and 24000000
> may not fit int type. Add explicit casting to prevent overflow.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.

You need to read and understand the code before writing any patches.
The '>> p' and '/ (m + 1)' are both just conditional 'divide by 2'.
So can be done before the multiply.
Since req->rate is 'signed long' and the value is a frequency it is
only just possible that it exceeds 31 bits (and will be wrong on 32bit
builds - but sun-9 might be 64bit only?)

In any case it would be sensible to force an unsigned divide.
So perhaps:
	unsigned int n = DIV_ROUND_UP(req->rate, 6000000ul);
	...
	req->rate = ((24000000ul >> p) / (m + 1)) * n;

David

> 
> Fixes: 6424e0aeebc4 ("clk: sunxi: rewrite sun9i_a80_get_pll4_factors()")
> Signed-off-by: Anastasia Belova <abelova@xxxxxxxxxxxxx>
> ---
>  drivers/clk/sunxi/clk-sun9i-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/sunxi/clk-sun9i-core.c b/drivers/clk/sunxi/clk-sun9i-core.c
> index d93c7a53c6c0..70fbd7390d96 100644
> --- a/drivers/clk/sunxi/clk-sun9i-core.c
> +++ b/drivers/clk/sunxi/clk-sun9i-core.c
> @@ -50,7 +50,7 @@ static void sun9i_a80_get_pll4_factors(struct factors_request *req)
>  	else if (n < 12)
>  		n = 12;
>  
> -	req->rate = ((24000000 * n) >> p) / (m + 1);
> +	req->rate = ((24000000ULL * n) >> p) / (m + 1);
>  	req->n = n;
>  	req->m = m;
>  	req->p = p;