git: c8c624de57b0 - stable/13 - dhclient: Keep two clocks

From: Colin Percival <cperciva_at_FreeBSD.org>
Date: Wed, 23 Apr 2025 04:51:00 UTC
The branch stable/13 has been updated by cperciva:

URL: https://cgit.FreeBSD.org/src/commit/?id=c8c624de57b072d14c1ab23167880a069ef51f11

commit c8c624de57b072d14c1ab23167880a069ef51f11
Author:     Colin Percival <cperciva@FreeBSD.org>
AuthorDate: 2025-04-08 22:50:01 +0000
Commit:     Colin Percival <cperciva@FreeBSD.org>
CommitDate: 2025-04-23 04:50:55 +0000

    dhclient: Keep two clocks
    
    Until July 2024, dhclient kept track of time as seconds-since-epoch as
    a time_t.  This was a problem because (a) we wanted sub-second timeouts
    and (b) timeouts didn't always do the right thing if the system clock
    changed.
    
    Switching to using CLOCK_MONOTONIC and struct timespec fixed those
    issues but introduced a new problem: CLOCK_MONOTONIC values were being
    intepreted as seconds-since-epoch and written to the dhclient.leases
    file, causing confusion with DHCP leases expiring in early 1970.
    
    Attempt to compromise between these by keeping track of both times;
    any type within dhclient which is a time_t now refers to seconds past
    the epoch, while any struct timespec value is a CLOCK_MONOTONIC time.
    
    PR:     283256
    Reviewed by:    dch
    Fixes:  f0a38976b01e ("dhclient: Use clock_gettime() instead of time()")
    Sponsored by:   Amazon
    Differential Revision:  https://reviews.freebsd.org/D49720
    
    (cherry picked from commit 43d19e6a4c42ade0f276ceca18a09e2e3829fce4)
---
 sbin/dhclient/dhclient.c | 50 +++++++++++++++++++++---------------------------
 sbin/dhclient/dhcpd.h    |  4 ++--
 sbin/dhclient/dispatch.c | 14 +++++++++-----
 3 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 141403d8c86b..0ac2a493a94f 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -90,8 +90,8 @@
 
 cap_channel_t *capsyslog;
 
-time_t cur_time;
-struct timespec time_now;
+time_t cur_time;	/* Seconds since epoch. */
+struct timespec time_now;	/* CLOCK_MONOTONIC. */
 static time_t default_lease_time = 43200; /* 12 hours... */
 
 const char *path_dhclient_conf = _PATH_DHCLIENT_CONF;
@@ -450,7 +450,7 @@ main(int argc, char *argv[])
 
 	tzset();
 	clock_gettime(CLOCK_MONOTONIC, &time_now);
-	cur_time = time_now.tv_sec;
+	cur_time = time(NULL);
 
 	inaddr_broadcast.s_addr = INADDR_BROADCAST;
 	inaddr_any.s_addr = INADDR_ANY;
@@ -1030,13 +1030,13 @@ dhcpoffer(struct packet *packet)
 	struct client_lease *lease, *lp;
 	int i;
 	struct timespec arp_timeout_needed;
-	struct timespec stop_selecting = { .tv_sec = 0, .tv_nsec = 0 };
-	time_now.tv_sec = cur_time;
-	time_now.tv_nsec = 0;
-
+	time_t stop_selecting;
+	struct timespec stop_time;
 	const char *name = packet->options[DHO_DHCP_MESSAGE_TYPE].len ?
 	    "DHCPOFFER" : "BOOTREPLY";
 
+	clock_gettime(CLOCK_MONOTONIC, &time_now);
+
 	/* If we're not receptive to an offer right now, or if the offer
 	   has an unrecognizable transaction id, then just drop it. */
 	if (ip->client->state != S_SELECTING ||
@@ -1096,7 +1096,7 @@ dhcpoffer(struct packet *packet)
 		arp_timeout_needed = arp_timeout;
 
 	/* Figure out when we're supposed to stop selecting. */
-	stop_selecting.tv_sec =
+	stop_selecting =
 	    ip->client->first_sending + ip->client->config->select_interval;
 
 	/* If this is the lease we asked for, put it at the head of the
@@ -1117,7 +1117,7 @@ dhcpoffer(struct packet *packet)
 		timespecadd(&time_now, &arp_timeout_needed, &interm_struct);
 
 		if (ip->client->offered_leases &&
-		    timespeccmp(&interm_struct, &stop_selecting, >))
+		    interm_struct.tv_sec >= stop_selecting)
 			arp_timeout_needed = zero_timespec;
 
 		/* Put the lease at the end of the list. */
@@ -1132,27 +1132,21 @@ dhcpoffer(struct packet *packet)
 		}
 	}
 
-	/* If we're supposed to stop selecting before we've had time
-	   to wait for the ARPREPLY, add some delay to wait for
-	   the ARPREPLY. */
-	struct timespec time_left;
-	timespecsub(&stop_selecting, &time_now, &time_left);
-
+	/*
+	 * Wait until stop_selecting seconds past the epoch, or until
+	 * arp_timeout_needed past now, whichever is longer.  Note that
+	 * the first case only occurs if select-timeout is set to nonzero
+	 * in dhclient.conf.
+	 */
+	struct timespec time_left =
+	    {.tv_sec = stop_selecting - cur_time, .tv_nsec = 0};
 	if (timespeccmp(&time_left, &arp_timeout_needed, <)) {
-		timespecadd(&time_now, &arp_timeout_needed, &stop_selecting);
-	}
-
-	/* If the selecting interval has expired, go immediately to
-	   state_selecting().  Otherwise, time out into
-	   state_selecting at the select interval. */
-
-
-	if (timespeccmp(&stop_selecting, &zero_timespec, <=))
-		state_selecting(ip);
-	else {
-		add_timeout_timespec(stop_selecting, state_selecting, ip);
-		cancel_timeout(send_discover, ip);
+		timespecadd(&time_now, &arp_timeout_needed, &stop_time);
+	} else {
+		timespecadd(&time_now, &time_left, &stop_time);
 	}
+	add_timeout_timespec(stop_time, state_selecting, ip);
+	cancel_timeout(send_discover, ip);
 }
 
 /* Allocate a client_lease structure and initialize it from the parameters
diff --git a/sbin/dhclient/dhcpd.h b/sbin/dhclient/dhcpd.h
index abe652c06fec..13c748c3f83f 100644
--- a/sbin/dhclient/dhcpd.h
+++ b/sbin/dhclient/dhcpd.h
@@ -361,8 +361,8 @@ char *piaddr(struct iaddr);
 extern cap_channel_t *capsyslog;
 extern const char *path_dhclient_conf;
 extern char *path_dhclient_db;
-extern struct timespec time_now;
-extern time_t cur_time;
+extern struct timespec time_now;	/* CLOCK_MONOTONIC */
+extern time_t cur_time;			/* Seconds since epoch */
 extern int log_priority;
 extern int log_perror;
 
diff --git a/sbin/dhclient/dispatch.c b/sbin/dhclient/dispatch.c
index 78d94804468b..bfa96b391bf8 100644
--- a/sbin/dhclient/dispatch.c
+++ b/sbin/dhclient/dispatch.c
@@ -171,8 +171,8 @@ dispatch(void)
 	struct protocol *l;
 	struct pollfd *fds;
 	struct timespec howlong;
-	time_now.tv_sec = cur_time;
-	time_now.tv_nsec = 0;
+
+	clock_gettime(CLOCK_MONOTONIC, &time_now);
 
 	for (l = protocols; l; l = l->next)
 		nfds++;
@@ -236,7 +236,7 @@ another:
 		if (count == -1) {
 			if (errno == EAGAIN || errno == EINTR) {
 				clock_gettime(CLOCK_MONOTONIC, &time_now);
-				cur_time = time_now.tv_sec;
+				cur_time = time(NULL);
 				continue;
 			} else
 				error("poll: %m");
@@ -244,7 +244,7 @@ another:
 
 		/* Get the current time... */
 		clock_gettime(CLOCK_MONOTONIC, &time_now);
-		cur_time = time_now.tv_sec;
+		cur_time = time(NULL);
 
 		i = 0;
 		for (l = protocols; l; l = l->next) {
@@ -377,7 +377,11 @@ active:
 void
 add_timeout(time_t when_s, void (*where)(void *), void *what)
 {
-	struct timespec when = { .tv_sec = when_s, .tv_nsec = 0 };
+	struct timespec when;
+
+	cur_time = time(NULL);
+	clock_gettime(CLOCK_MONOTONIC, &when);
+	when.tv_sec += when_s - cur_time;
 	add_timeout_timespec(when, where, what);
 }