[PATCH] media: i2c: ov5640: Fix potential integer overflow in sysclk calculation

[PATCH] media: i2c: ov5640: Fix potential integer overflow in sysclk calculation

[Agalakov Daniil]

The calculation of sysclk uses 32-bit arithmetic because
sensor->xclk_freq is a 32-bit integer. The intermediate multiplication
result can overflow 32 bits sysclk variable.

For example, with pll_prediv fixed at 3 (OV5640_PLL_PREDIV), xclk_freq
set to its maximum of 54MHz (OV5640_XCLK_MAX) and a pll_mult value above
238 (the maximum value for pll_mult is 252, defined as
OV5640_PLL_MULT_MAX), the result exceeds the 32-bit limit.

This overflow causes the 1GHz safety check to fail, as the truncated
value fits within the 1GHz limit. Consequently, the function returns an
incorrect frequency, leading to misconfiguration of the sensor.

Explicitly cast the first operand to u64 to ensure the entire expression
is evaluated in 64-bit precision.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: aa2882481cad ("media: ov5640: Adjust the clock based on the expected rate")
Signed-off-by: Agalakov Daniil <ade@amicon.ru>
---
 drivers/media/i2c/ov5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 85ecc23b3587..04e5a683976a 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1377,7 +1377,7 @@ static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
 					    u8 pll_prediv, u8 pll_mult,
 					    u8 sysdiv)
 {
-	unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
+	u64 sysclk = (u64)sensor->xclk_freq / pll_prediv * pll_mult;
 
 	/* PLL1 output cannot exceed 1GHz. */
 	if (sysclk / 1000000 > 1000)
-- 
2.51.0

Re: [PATCH] media: i2c: ov5640: Fix potential integer overflow in sysclk calculation

[David Laight]

On Mon, 20 Apr 2026 18:39:50 +0300
Agalakov Daniil <ade@amicon.ru> wrote:

> The calculation of sysclk uses 32-bit arithmetic because
> sensor->xclk_freq is a 32-bit integer. The intermediate multiplication
> result can overflow 32 bits sysclk variable.
> 
> For example, with pll_prediv fixed at 3 (OV5640_PLL_PREDIV), xclk_freq
> set to its maximum of 54MHz (OV5640_XCLK_MAX) and a pll_mult value above
> 238 (the maximum value for pll_mult is 252, defined as
> OV5640_PLL_MULT_MAX), the result exceeds the 32-bit limit.
> 
> This overflow causes the 1GHz safety check to fail, as the truncated
> value fits within the 1GHz limit. Consequently, the function returns an
> incorrect frequency, leading to misconfiguration of the sensor.
> 
> Explicitly cast the first operand to u64 to ensure the entire expression
> is evaluated in 64-bit precision.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: aa2882481cad ("media: ov5640: Adjust the clock based on the expected rate")
> Signed-off-by: Agalakov Daniil <ade@amicon.ru>
> ---
>  drivers/media/i2c/ov5640.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 85ecc23b3587..04e5a683976a 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1377,7 +1377,7 @@ static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
>  					    u8 pll_prediv, u8 pll_mult,
>  					    u8 sysdiv)
>  {
> -	unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
> +	u64 sysclk = (u64)sensor->xclk_freq / pll_prediv * pll_mult;
>  
>  	/* PLL1 output cannot exceed 1GHz. */
>  	if (sysclk / 1000000 > 1000)

It would be much better to rework the test to avoid a 64bit divide
(which will fail to compile on 32bit).
Even the 'sysclk / 1000000 > 1000' test may fail to compile,
the compiler might convert it so the equivalent 'sysclk > 1000999999'
to avoid the division.
I don't know if that is the correct test - it doesn't match the comment.

You could do something like:
	u32 sysclk;
	if ((sensor->xclk_freq >> 8) * pll_mult > (2000000000 >> 8) * pll_prediv)
		/* output would be over 2GHz (or thereabouts) */
		...
	sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
	if (sysclk > 1000000000)
		/* output is over 1GHz */

	David