Re: [PATCH v2] mm,writeback: fix divide by zero in pos_ratio_polynom
From: Rik van Riel
Date: Wed Apr 30 2014 - 11:31:20 EST
On 04/30/2014 10:49 AM, Michal Hocko wrote:
On Wed 30-04-14 10:31:29, Rik van Riel wrote:
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;
How is that any different than defining divisor as 32b directly?
For unsigned, it probably doesn't make a difference.
For signed int vs unsigned long, I wonder if there is a
corner case where casting the second to the first before
doing the "limit - setpoint" calculation can lead to a
different outcome...
--
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/