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 <[email protected]> > >>--- > >> 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? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

