Control: tag -1 patch
On Sun, 2012-12-16 at 07:21 +1100, paul.szabo@sydney.edu.au wrote:
> Seems to me that the bug is in function
> bdi_position_ratio()
> within file
> mm/page-writeback.c
> The internal variable declaration is
> long long pos_ratio;
> and calculation of it overflows.
You seem to be on the right track, but I think the initial overflow
occurs when calculating x:
[...]
> 562 x = div_s64((setpoint - dirty) << RATELIMIT_CALC_SHIFT,
> 563 limit - setpoint + 1);
[...]
setpoint and dirty are numbers of pages and are declared as long, so on
a system with enough memory they can presumably differ by 2^21 or more
(2^21 pages = 8 GB). Shifting left by RATELIMIT_CALC_SHIFT = 10 can
then change the sign bit.
Does the attached patch fix this?
Ben.
--
Ben Hutchings
Theory and practice are closer in theory than in practice.
- John Levine, moderator of comp.compilers
From 8f4ae99695a37c7294ce39e008878e8b304cb4b0 Mon Sep 17 00:00:00 2001 From: Ben Hutchings <ben@decadent.org.uk> Date: Sat, 15 Dec 2012 23:03:40 +0000 Subject: [PATCH] writeback: Fix overflow in bdi_position_ratio() on PAE systems Most variables in bdi_position_ratio() are declared long, which is enough for a page count. However, when converting (setpoint - dirty) to a fixed-point number we left-shift by 10, and on a 32-bit system with PAE it is possible to have enough dirty pages that the shift overflows into the sign bit. We need to cast to s64 before the left-shift. Reported-by: Paul Szabo <paul.szabo@sydney.edu.au> Reference: http://bugs.debian.org/695182 Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- mm/page-writeback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 50f0824..8b5600e 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -559,7 +559,7 @@ static unsigned long bdi_position_ratio(struct backing_dev_info *bdi, * => fast response on large errors; small oscillation near setpoint */ setpoint = (freerun + limit) / 2; - x = div_s64((setpoint - dirty) << RATELIMIT_CALC_SHIFT, + x = div_s64((s64)(setpoint - dirty) << RATELIMIT_CALC_SHIFT, limit - setpoint + 1); pos_ratio = x; pos_ratio = pos_ratio * x >> RATELIMIT_CALC_SHIFT;
Attachment:
signature.asc
Description: This is a digitally signed message part