Messages in this thread | | | Date | Mon, 17 Aug 2009 13:20:50 +0200 (CEST) | Subject | Re: [PATCH] poll/select: avoid arithmetic overflow in __estimate_accuracy() | From | "Guillaume Knispel" <> |
| |
> 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@proformatique.com> >>--- >> 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
| |