sched: Don't cast to int32_t the monotime_diff_msec() result

When the KIST schedule() is called, it computes a diff value between the last
scheduler run and the current monotonic time. If tha value is below the run
interval, the libevent even is updated else the event is run.

It turned out that casting to int32_t the returned int64_t value for the very
first scheduler run (which is set to 0) was creating an overflow on the 32 bit
value leading to adding the event with a gigantic usec value. The scheduler
was simply never running for a while.

First of all, a BUG() is added for a diff value < 0 because if the time is
really monotonic, we should never have a now time that is lower than the last
scheduler run time. And we will try to recover with a diff value to 0.

Second, the diff value is changed to int64_t so we avoid this "bootstrap
overflow" and future casting overflow problems.

Fixes #23558

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c
index ba8d416dbad77b1d72c61f2baabd99ea4bf84ecf..e4ae071072dfebf9ee6a5e8184ccec456912e759 100644 (file)
--- a/src/or/scheduler_kist.c
+++ b/src/or/scheduler_kist.c
@@ -507,16 +507,25 @@ kist_scheduler_schedule(void)
 {
   struct monotime_t now;
   struct timeval next_run;
-  int32_t diff;
+  int64_t diff;
 
   if (!have_work()) {
     return;
   }
   monotime_get(&now);
-  diff = (int32_t) monotime_diff_msec(&scheduler_last_run, &now);
+
+  /* If time is really monotonic, we can never have now being smaller than the
+   * last scheduler run. The scheduler_last_run at first is set to 0. */
+  diff = monotime_diff_msec(&scheduler_last_run, &now);
+  IF_BUG_ONCE(diff < 0) {
+    diff = 0;
+  }
   if (diff < sched_run_interval) {
     next_run.tv_sec = 0;
-    /* 1000 for ms -> us */
+    /* Takes 1000 ms -> us. This will always be valid because diff can NOT be
+     * negative and can NOT be smaller than sched_run_interval so values can
+     * only go from 1000 usec (diff set to interval - 1) to 100000 usec (diff
+     * set to 0) for the maximum allowed run interval (100ms). */
     next_run.tv_usec = (sched_run_interval - diff) * 1000;
     /* Readding an event reschedules it. It does not duplicate it. */
     scheduler_ev_add(&next_run);