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

Rik van Riel
2014-04-30T14:34:10Z

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@redhat.com>
>> ---
>> 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;