On Fri, 21 May 2021 14:19:53 +0800 tq17373059@xxxxxxxxxxx wrote: > From: Qi Teng <tq17373059@xxxxxxxxxxx> > > 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 <oslab@xxxxxxxxxxxxxxx> > Signed-off-by: Qi Teng <tq17373059@xxxxxxxxxxx> 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 > --- > 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); >
