Re: [PATCH] iio: adf4350: fix a possible divided-by-zero bug in adf4350_set_freq()
From: Jonathan Cameron <jic23@kernel.org>
Date: 2021-05-21 17:24:13
Also in:
lkml
On Fri, 21 May 2021 14:19:53 +0800 tq17373059@buaa.edu.cn wrote:
From: Qi Teng <redacted> The variable st->r1_mod is checked in: if (st->r0_fract && st->r1_mod) This indicates that st->r1_mod can be zero. Its value is the same as that in: st->r0_fract = do_div(tmp, st->r1_mod); However, st->r1_mod performs as a divisor in this statement, which implies a possible divided-by-zero bug. To fix this possible bug, st->r1_mod is checked before the division operation. If it is zero, st->r0_fract is set to zero instead of do_div(tmp, st->r1_mod). Reported-by: TOTE Robot <redacted> Signed-off-by: Qi Teng <redacted>
Patch seems to be inverse of the intended. You are removing a divide by 0 protection that isn't present in the current driver. Also, a fix like this needs a Fixes: tag. The maths is sufficiently messy that I can't immediately tell if this can actually be zero or whether the check you are highlighting is paranoia. Given that, I agree that a fix here makes sense whether or not we have a verified bug. Thanks, Jonathan
quoted hunk ↗ jump to hunk
--- drivers/iio/frequency/adf4350.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c index 99c6f260cc21..1462a6a5bc6d 100644 --- a/drivers/iio/frequency/adf4350.c +++ b/drivers/iio/frequency/adf4350.c@@ -182,10 +182,7 @@ static int adf4350_set_freq(struct adf4350_state *st, unsigned long long freq) tmp = freq * (u64)st->r1_mod + (st->fpfd >> 1); do_div(tmp, st->fpfd); /* Div round closest (n + d/2)/d */ - if (st->r1_mod) - st->r0_fract = do_div(tmp, st->r1_mod); - else - st->r0_fract = 0; + st->r0_fract = do_div(tmp, st->r1_mod); st->r0_int = tmp; } while (mdiv > st->r0_int);