Re: [PATCH 23/35] media: dvb-frontends: tda10048: Use the right div

Dan Carpenter
2024-04-16T10:10:11Z

On Mon, Apr 15, 2024 at 07:34:40PM +0000, Ricardo Ribalda wrote:
> z does not fit in 32 bits.
>

z has to fit in 32 bits otherwise there is a different bug.

> Found by cocci:
> drivers/media/dvb-frontends/tda10048.c:345:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/dvb-frontends/tda10048.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/tda10048.c b/drivers/media/dvb-frontends/tda10048.c
> index 5d5e4e9e4422..b176e7803e5b 100644
> --- a/drivers/media/dvb-frontends/tda10048.c
> +++ b/drivers/media/dvb-frontends/tda10048.c
> @@ -342,8 +342,7 @@ static int tda10048_set_wref(struct dvb_frontend *fe, u32 sample_freq_hz,
> t *= (2048 * 1024);
> t *= 1024;
> z = 7 * sample_freq_hz;

sample_freq_hz is a u32 so z can't be more than U32_MAX. Perhaps there
is an integer overflow bug on this line.

The sample frequency is set in tda10048_set_if().

state->sample_freq = state->xtal_hz * (state->pll_mfactor + 45);

->xtal_hz is set earlier in tda10048_set_if() and it goes up to
16,000,000. So if ->pll_mfactor is non-zero this line will have an
integer overflow. 16million * 46 > U32_MAX. Maybe when .clk_freq_khz
is TDA10048_CLK_16000 then ->pll_mfactor is zero? Ugh...

> - do_div(t, z);
> - t += 5;
> + t = div64_u64(t, z) + 5;
> do_div(t, 10);

regards,
dan carpenter