Re: [PATCH v2] mm,writeback: fix divide by zero in pos_ratio_polynom

[Rik van Riel]

On 04/30/2014 09:48 AM, Michal Hocko wrote:
> On Wed 30-04-14 09:30:35, Rik van Riel wrote:
> [...]
> > Subject: mm,writeback: fix divide by zero in pos_ratio_polynom
> >
> > It is possible for "limit - setpoint + 1" to equal zero, leading to a
> > divide by zero error. Blindly adding 1 to "limit - setpoint" is not
> > working, so we need to actually test the divisor before calling div64.
> >
> > Signed-off-by: Rik van Riel <riel@xxxxxxxxxx>
> > ---
> >   mm/page-writeback.c | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> > index ef41349..f98a297 100644
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -597,11 +597,16 @@ static inline long long pos_ratio_polynom(unsigned long setpoint,
> >   					  unsigned long dirty,
> >   					  unsigned long limit)
> >   {
> > +	unsigned long divisor;
> >   	long long pos_ratio;
> >   	long x;
> >
> > +	divisor = limit - setpoint;
> > +	if (!divisor)
> > +		divisor = 1;	/* Avoid div-by-zero */
> > +
>
> This is still prone to u64 -> s32 issue, isn't it?
> What was the original problem anyway? Was it really setpoint > limit or
> rather the overflow?

Thinking about it some more, is it possible that
limit and/or setpoint are larger than 32 bits, but
the difference between them is not?

In that case, truncating both to 32 bits before
doing the subtraction would be troublesome, and
it would be better to do a cast in the comparison:

if (!(s32)divisor)
	divisor = 1;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/