Permalink
Browse files

unix: make timers handle large timeouts

This commit fixes two closely related integer overflow bugs:

* Timers with a timeout > INT_MAX cause uv__next_timeout() to return
  a negative value.

* Timers with very large timeouts (close or equal to ULLONG_MAX) run on
  the next tick.

In both cases, clamp the values to prevent the overflow from happening.

Fixes nodejs/node-v0.x-archive#5101.
  • Loading branch information...
1 parent 77cb29a commit 9b619396d93182be26287f261ac654611aa99d95 @bnoordhuis bnoordhuis committed Mar 21, 2013
Showing with 70 additions and 3 deletions.
  1. +15 −2 src/unix/timer.c
  2. +4 −0 test/test-list.h
  3. +51 −1 test/test-timer.c
View
@@ -21,6 +21,8 @@
#include "uv.h"
#include "internal.h"
#include <assert.h>
+#include <limits.h>
+
static int uv__timer_cmp(const uv_timer_t* a, const uv_timer_t* b) {
if (a->timeout < b->timeout)
@@ -55,11 +57,17 @@ int uv_timer_start(uv_timer_t* handle,
uv_timer_cb cb,
uint64_t timeout,
uint64_t repeat) {
+ uint64_t clamped_timeout;
+
if (uv__is_active(handle))
uv_timer_stop(handle);
+ clamped_timeout = handle->loop->time + timeout;
+ if (clamped_timeout < timeout)
+ clamped_timeout = (uint64_t) -1;
+
handle->timer_cb = cb;
- handle->timeout = handle->loop->time + timeout;
+ handle->timeout = clamped_timeout;
handle->repeat = repeat;
/* start_id is the second index to be compared in uv__timer_cmp() */
handle->start_id = handle->loop->timer_counter++;
@@ -107,6 +115,7 @@ uint64_t uv_timer_get_repeat(const uv_timer_t* handle) {
int uv__next_timeout(const uv_loop_t* loop) {
const uv_timer_t* handle;
+ uint64_t diff;
/* RB_MIN expects a non-const tree root. That's okay, it doesn't modify it. */
handle = RB_MIN(uv__timers, (struct uv__timers*) &loop->timer_handles);
@@ -117,7 +126,11 @@ int uv__next_timeout(const uv_loop_t* loop) {
if (handle->timeout <= loop->time)
return 0;
- return handle->timeout - loop->time;
+ diff = handle->timeout - loop->time;
+ if (diff > INT_MAX)
+ diff = INT_MAX;
+
+ return diff;
}
View
@@ -99,6 +99,8 @@ TEST_DECLARE (timer_init)
TEST_DECLARE (timer_again)
TEST_DECLARE (timer_start_twice)
TEST_DECLARE (timer_order)
+TEST_DECLARE (timer_huge_timeout)
+TEST_DECLARE (timer_huge_repeat)
TEST_DECLARE (idle_starvation)
TEST_DECLARE (loop_handles)
TEST_DECLARE (get_loadavg)
@@ -341,6 +343,8 @@ TASK_LIST_START
TEST_ENTRY (timer_again)
TEST_ENTRY (timer_start_twice)
TEST_ENTRY (timer_order)
+ TEST_ENTRY (timer_huge_timeout)
+ TEST_ENTRY (timer_huge_repeat)
TEST_ENTRY (idle_starvation)
View
@@ -28,8 +28,10 @@ static int once_close_cb_called = 0;
static int repeat_cb_called = 0;
static int repeat_close_cb_called = 0;
static int order_cb_called = 0;
-
static uint64_t start_time;
+static uv_timer_t tiny_timer;
+static uv_timer_t huge_timer1;
+static uv_timer_t huge_timer2;
static void once_close_cb(uv_handle_t* handle) {
@@ -212,5 +214,53 @@ TEST_IMPL(timer_order) {
ASSERT(order_cb_called == 2);
+ MAKE_VALGRIND_HAPPY();
+ return 0;
+}
+
+
+static void tiny_timer_cb(uv_timer_t* handle, int status) {
+ ASSERT(handle == &tiny_timer);
+ uv_close((uv_handle_t*) &tiny_timer, NULL);
+ uv_close((uv_handle_t*) &huge_timer1, NULL);
+ uv_close((uv_handle_t*) &huge_timer2, NULL);
+}
+
+
+TEST_IMPL(timer_huge_timeout) {
+ ASSERT(0 == uv_timer_init(uv_default_loop(), &tiny_timer));
+ ASSERT(0 == uv_timer_init(uv_default_loop(), &huge_timer1));
+ ASSERT(0 == uv_timer_init(uv_default_loop(), &huge_timer2));
+ ASSERT(0 == uv_timer_start(&tiny_timer, tiny_timer_cb, 1, 0));
+ ASSERT(0 == uv_timer_start(&huge_timer1, tiny_timer_cb, 0xffffffffffffLL, 0));
+ ASSERT(0 == uv_timer_start(&huge_timer2, tiny_timer_cb, (uint64_t) -1, 0));
+ ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+ MAKE_VALGRIND_HAPPY();
+ return 0;
+}
+
+
+static void huge_repeat_cb(uv_timer_t* handle, int status) {
+ static int ncalls;
+
+ if (ncalls == 0)
+ ASSERT(handle == &huge_timer1);
+ else
+ ASSERT(handle == &tiny_timer);
+
+ if (++ncalls == 10) {
+ uv_close((uv_handle_t*) &tiny_timer, NULL);
+ uv_close((uv_handle_t*) &huge_timer1, NULL);
+ }
+}
+
+
+TEST_IMPL(timer_huge_repeat) {
+ ASSERT(0 == uv_timer_init(uv_default_loop(), &tiny_timer));
+ ASSERT(0 == uv_timer_init(uv_default_loop(), &huge_timer1));
+ ASSERT(0 == uv_timer_start(&tiny_timer, huge_repeat_cb, 2, 2));
+ ASSERT(0 == uv_timer_start(&huge_timer1, huge_repeat_cb, 1, (uint64_t) -1));
+ ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+ MAKE_VALGRIND_HAPPY();
return 0;
}

0 comments on commit 9b61939

Please sign in to comment.