From: Willy Tarreau <w@1wt.eu>
Date: Fri, 15 Nov 2024 14:52:17 +0000 (+0100)
Subject: BUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY
X-Git-Tag: v3.1-dev13~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5a3735a155d47786c20774a1492f3ed20cfe4df3;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: clock: make sure now_ms cannot be TICK_ETERNITY

In clock ticks, 0 is TICK_ETERNITY. Long ago we used to make sure now_ms
couldn't be zero so that it could be assigned to expiration timers, but
it has long changed after functions like tick_add() were instrumented to
make the check. The problem is that aside the rare few accidental direct
assignments to expiration dates, it's also used to mark the beginning of
an event that's later checked against TICK_ETERNITY to know if it has
already struck. The problem in this case is that certain events may just
be replaced or dropped just because they apparently never appeared. It's
probably the case for stconn's "lra" and "fsb" fields, just like it is
for all those involving tick_add_ifset(), like h2c->idle_start.

The right approach would be to change the type of now_ms to something
else that cannot take direct computations and that represents a timestamp,
forcing to always use the conversion functions. The variables holding such
timestamps would also be distinguished from intervals. At first glance we
could have for timestamps:
  - 0 = never happened (for the past), eternity (for the future)
  - X = date
and for intervals:
  - 0 = not set
  - X = interval

However this requires significant changes. Instead for now, let's just
make sure again that now_ms is never 0 by setting it to 1 when this
happens (1 / 4 billion times, or 1ms every 49.7 days).

This will need to be carefully backported to older versions. Note that
with this patch backported, the previous ones fixing the zero date are
not strictly needed.
---

diff --git a/src/clock.c b/src/clock.c
index f230a56f1b..1a6a80be00 100644
--- a/src/clock.c
+++ b/src/clock.c
@@ -254,6 +254,10 @@ void clock_update_local_date(int max_wait, int interrupted)
 		now_ns = date_ns + ofs;
 	}
 	now_ms = ns_to_ms(now_ns);
+
+	/* correct for TICK_ETNERITY (0) */
+	if (unlikely(now_ms == TICK_ETERNITY))
+		now_ms++;
 }
 
 void clock_update_global_date()
@@ -285,6 +289,9 @@ void clock_update_global_date()
 		 * what we're doing here.
 		 */
 		now_ms = ns_to_ms(now_ns);
+		/* correct for TICK_ETNERITY (0) */
+		if (unlikely(now_ms == TICK_ETERNITY))
+			now_ms++;
 
 		if (!((now_ns ^ old_now_ns) & ~0x7FFFULL))
 			return;
@@ -330,7 +337,11 @@ void clock_init_process_date(void)
 	now_offset = sec_to_ns((uint)((uint)(-global_now_ms) / 1000U - BOOT_TIME_WRAP_SEC));
 	global_now_ns += now_offset;
 	now_ns = global_now_ns;
-	now_ms = global_now_ms = ns_to_ms(now_ns);
+	now_ms = ns_to_ms(now_ns);
+	/* correct for TICK_ETNERITY (0) */
+	if (now_ms == TICK_ETERNITY)
+		now_ms++;
+	global_now_ms = now_ms;
 
 	th_ctx->idle_pct = 100;
 	clock_update_date(0, 1);