> On Sun, Aug 16, 2009 at 10:29:21PM +0200, Guillaume Knispel wrote: >>__estimate_accuracy() was prone to integer overflow, for example >>if *tv == {2147, 483648000} on a 32 bit computer (or even for delays >>as small as {429, 500000000} if the task is niced). >> >>Because the result was already forced between 0 and 100ms, the effect >>of the overflow was not too problematic, but the use of the hrtimer >>range feature was not optimal in overflow cases. >> >>This patch ensures that there can not be an integer overflow in this >>function. >> >>Signed-off-by: Guillaume Knispel <gknispel@xxxxxxxxxxxxxxxxx> >>--- >> fs/select.c | 14 ++++++++++---- >> 1 files changed, 10 insertions(+), 4 deletions(-) >> >>diff --git a/fs/select.c b/fs/select.c >>index 8084834..a201fc3 100644 >>--- a/fs/select.c >>+++ b/fs/select.c >>@@ -41,22 +41,28 @@ >> * better solutions.. >> */ >> >>+#define MAX_SLACK (100 * NSEC_PER_MSEC) >>+ >> static long __estimate_accuracy(struct timespec *tv) >> { >> long slack; >> int divfactor = 1000; >> >>+ if (tv->tv_sec < 0) >>+ return 0; >>+ >> if (task_nice(current) > 0) >> divfactor = divfactor / 5; >> >>+ if (tv->tv_sec > MAX_SLACK / (NSEC_PER_SEC/divfactor)) >>+ return MAX_SLACK; >>+ > > > Yes? Isn't MAX_SLACK also too big for 'long' on 32bit machine? > > Try: > > % printf "%d\n" "100*1000000000 > 0x7fffffff" > 1 > > Am I missing something here? 100 * NSEC_PER_MSEC => 100 * 1000000 - The exact same multiplication was already in the existing code, and this is not the one that can overflow. Cheers, Guillaume Knispel -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html