ndisc: cleanup handling of timestamps

We have the timestamp nm_utils_get_monotonic_time_s(), which should be
gint32 type. Then we also have timestamps in the NMNDisc* objects, which
consist of guint32 timestamp and lifetime.

Cleanup handling the times and calculation of the timestamps by using
the correct integer type consistently and ensuring that no integer overflow
occurs.

5 files changed, 81 insertions, 44 deletions

diff --git a/src/ndisc/nm-fake-ndisc.c b/src/ndisc/nm-fake-ndisc.c
index 7a9fb11079..421fc53c53 100644
--- a/src/ndisc/nm-fake-ndisc.c
+++ b/src/ndisc/nm-fake-ndisc.c
@@ -250,7 +250,7 @@ receive_ra (gpointer user_data)
 	NMNDiscDataInternal *rdata = ndisc->rdata;
 	FakeRa *ra = priv->ras->data;
 	NMNDiscConfigMap changed = 0;
-	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	gint32 now = nm_utils_get_monotonic_timestamp_s ();
 	guint i;
 	NMNDiscDHCPLevel dhcp_level;
 
diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c
index 3bc1590ea8..f33c48340f 100644
--- a/src/ndisc/nm-lndp-ndisc.c
+++ b/src/ndisc/nm-lndp-ndisc.c
@@ -108,7 +108,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 	NMNDiscConfigMap changed = 0;
 	struct ndp_msgra *msgra = ndp_msgra (msg);
 	struct in6_addr gateway_addr;
-	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	gint32 now = nm_utils_get_monotonic_timestamp_s ();
 	int offset;
 	int hop_limit;
 
@@ -123,7 +123,7 @@ receive_ra (struct ndp *ndp, struct ndp_msg *msg, gpointer user_data)
 	 * single time when the configuration is finished and updates can
 	 * come at any time.
 	 */
-	_LOGD ("received router advertisement at %u", now);
+	_LOGD ("received router advertisement at %d", (int) now);
 
 	/* DHCP level:
 	 *
@@ -337,7 +337,7 @@ send_ra (NMNDisc *ndisc, GError **error)
 {
 	NMLndpNDiscPrivate *priv = NM_LNDP_NDISC_GET_PRIVATE ((NMLndpNDisc *) ndisc);
 	NMNDiscDataInternal *rdata = ndisc->rdata;
-	guint32 now = nm_utils_get_monotonic_timestamp_s ();
+	gint32 now = nm_utils_get_monotonic_timestamp_s ();
 	int errsv;
 	struct in6_addr *addr;
 	struct ndp_msg *msg;
@@ -367,14 +367,14 @@ send_ra (NMNDisc *ndisc, GError **error)
 	 * whose prefixes are suitable for delegating. Let's announce them. */
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *address = &g_array_index (rdata->addresses, NMNDiscAddress, i);
-		guint32 age = now - address->timestamp;
+		guint32 age = NM_CLAMP ((gint64) now - (gint64) address->timestamp, 0, G_MAXUINT32 - 1);
 		guint32 lifetime = address->lifetime;
 		guint32 preferred = address->preferred;
 
 		/* Clamp the life times if they're not forever. */
-		if (lifetime != 0xffffffff)
+		if (lifetime != NM_NDISC_INFINITY)
 			lifetime = lifetime > age ? lifetime - age : 0;
-		if (preferred != 0xffffffff)
+		if (preferred != NM_NDISC_INFINITY)
 			preferred = preferred > age ? preferred - age : 0;
 
 		prefix = _ndp_msg_add_option (msg, sizeof(*prefix));
diff --git a/src/ndisc/nm-ndisc-private.h b/src/ndisc/nm-ndisc-private.h
index 10bcc64f27..bbecb01a43 100644
--- a/src/ndisc/nm-ndisc-private.h
+++ b/src/ndisc/nm-ndisc-private.h
@@ -36,7 +36,7 @@ struct _NMNDiscDataInternal {
 
 typedef struct _NMNDiscDataInternal NMNDiscDataInternal;
 
-void nm_ndisc_ra_received (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap changed);
+void nm_ndisc_ra_received (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed);
 void nm_ndisc_rs_received (NMNDisc *ndisc);
 
 gboolean nm_ndisc_add_gateway              (NMNDisc *ndisc, const NMNDiscGateway *new);
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 936b0bf3a3..cc740c4aa7 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -519,7 +519,8 @@ static void
 solicit_routers (NMNDisc *ndisc)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
-	gint64 next, now;
+	gint32 now, next;
+	gint64 t;
 
 	if (priv->send_rs_id)
 		return;
@@ -527,9 +528,9 @@ solicit_routers (NMNDisc *ndisc)
 	now = nm_utils_get_monotonic_timestamp_s ();
 	priv->solicitations_left = priv->router_solicitations;
 
-	next = (((gint64) priv->last_rs) + priv->router_solicitation_interval) - now;
-	next = CLAMP (next, 0, G_MAXINT32);
-	_LOGD ("scheduling explicit router solicitation request in %" G_GINT64_FORMAT " seconds.",
+	t = (((gint64) priv->last_rs) + priv->router_solicitation_interval) - now;
+	next = CLAMP (t, 0, G_MAXINT32);
+	_LOGD ("scheduling explicit router solicitation request in %" G_GINT32_FORMAT " seconds.",
 	       next);
 	priv->send_rs_id = g_timeout_add_seconds ((guint32) next, (GSourceFunc) send_rs_timeout, ndisc);
 }
@@ -797,7 +798,39 @@ dhcp_level_to_string (NMNDiscDHCPLevel dhcp_level)
 	}
 }
 
-#define expiry(item) (item->timestamp + item->lifetime)
+static gint32
+get_expiry_time (guint32 timestamp, guint32 lifetime)
+{
+	gint64 t;
+
+	/* timestamp is supposed to come from nm_utils_get_monotonic_timestamp_s().
+	 * It is expected to be within a certain range. */
+	nm_assert (timestamp > 0);
+	nm_assert (timestamp <= G_MAXINT32);
+
+	if (lifetime == NM_NDISC_INFINITY)
+		return G_MAXINT32;
+
+	t = (gint64) timestamp + (gint64) lifetime;
+	return CLAMP (t, 0, G_MAXINT32 - 1);
+}
+
+#define get_expiry(item) \
+	({ \
+		typeof (item) _item = (item); \
+		nm_assert (_item); \
+		get_expiry_time ((_item->timestamp), (_item->lifetime)); \
+	})
+
+#define get_expiry_half(item) \
+	({ \
+		typeof (item) _item = (item); \
+		nm_assert (_item); \
+		get_expiry_time ((_item->timestamp),\
+		                 (_item->lifetime) == NM_NDISC_INFINITY \
+		                   ? NM_NDISC_INFINITY \
+		                   : (_item->lifetime) / 2); \
+	})
 
 static void
 _config_changed_log (NMNDisc *ndisc, NMNDiscConfigMap changed)
@@ -821,37 +854,37 @@ _config_changed_log (NMNDisc *ndisc, NMNDiscConfigMap changed)
 		NMNDiscGateway *gateway = &g_array_index (rdata->gateways, NMNDiscGateway, i);
 
 		inet_ntop (AF_INET6, &gateway->address, addrstr, sizeof (addrstr));
-		_LOGD ("  gateway %s pref %d exp %u", addrstr, gateway->preference, expiry (gateway));
+		_LOGD ("  gateway %s pref %d exp %d", addrstr, gateway->preference, get_expiry (gateway));
 	}
 	for (i = 0; i < rdata->addresses->len; i++) {
 		NMNDiscAddress *address = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
 		inet_ntop (AF_INET6, &address->address, addrstr, sizeof (addrstr));
-		_LOGD ("  address %s exp %u", addrstr, expiry (address));
+		_LOGD ("  address %s exp %d", addrstr, get_expiry (address));
 	}
 	for (i = 0; i < rdata->routes->len; i++) {
 		NMNDiscRoute *route = &g_array_index (rdata->routes, NMNDiscRoute, i);
 
 		inet_ntop (AF_INET6, &route->network, addrstr, sizeof (addrstr));
-		_LOGD ("  route %s/%u via %s pref %d exp %u", addrstr, (guint) route->plen,
+		_LOGD ("  route %s/%u via %s pref %d exp %d", addrstr, (guint) route->plen,
 		       nm_utils_inet6_ntop (&route->gateway, NULL), route->preference,
-		       expiry (route));
+		       get_expiry (route));
 	}
 	for (i = 0; i < rdata->dns_servers->len; i++) {
 		NMNDiscDNSServer *dns_server = &g_array_index (rdata->dns_servers, NMNDiscDNSServer, i);
 
 		inet_ntop (AF_INET6, &dns_server->address, addrstr, sizeof (addrstr));
-		_LOGD ("  dns_server %s exp %u", addrstr, expiry (dns_server));
+		_LOGD ("  dns_server %s exp %d", addrstr, get_expiry (dns_server));
 	}
 	for (i = 0; i < rdata->dns_domains->len; i++) {
 		NMNDiscDNSDomain *dns_domain = &g_array_index (rdata->dns_domains, NMNDiscDNSDomain, i);
 
-		_LOGD ("  dns_domain %s exp %u", dns_domain->domain, expiry (dns_domain));
+		_LOGD ("  dns_domain %s exp %d", dns_domain->domain, get_expiry (dns_domain));
 	}
 }
 
 static void
-clean_gateways (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *nextevent)
+clean_gateways (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -861,8 +894,8 @@ clean_gateways (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32
 	for (i = 0; i < rdata->gateways->len; ) {
 		NMNDiscGateway *item = &g_array_index (rdata->gateways, NMNDiscGateway, i);
 
-		if (item->lifetime != G_MAXUINT32) {
-			guint64 expiry = (guint64) item->timestamp + item->lifetime;
+		if (item->lifetime != NM_NDISC_INFINITY) {
+			gint32 expiry = get_expiry (item);
 
 			if (now >= expiry) {
 				g_array_remove_index (rdata->gateways, i);
@@ -877,7 +910,7 @@ clean_gateways (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32
 }
 
 static void
-clean_addresses (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *nextevent)
+clean_addresses (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -887,8 +920,8 @@ clean_addresses (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32
 	for (i = 0; i < rdata->addresses->len; ) {
 		NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
 
-		if (item->lifetime != G_MAXUINT32) {
-			guint64 expiry = (guint64) item->timestamp + item->lifetime;
+		if (item->lifetime != NM_NDISC_INFINITY) {
+			gint32 expiry = get_expiry (item);
 
 			if (now >= expiry) {
 				g_array_remove_index (rdata->addresses, i);
@@ -903,7 +936,7 @@ clean_addresses (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32
 }
 
 static void
-clean_routes (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *nextevent)
+clean_routes (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -913,8 +946,8 @@ clean_routes (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *n
 	for (i = 0; i < rdata->routes->len; ) {
 		NMNDiscRoute *item = &g_array_index (rdata->routes, NMNDiscRoute, i);
 
-		if (item->lifetime != G_MAXUINT32) {
-			guint64 expiry = (guint64) item->timestamp + item->lifetime;
+		if (item->lifetime != NM_NDISC_INFINITY) {
+			gint32 expiry = get_expiry (item);
 
 			if (now >= expiry) {
 				g_array_remove_index (rdata->routes, i);
@@ -929,7 +962,7 @@ clean_routes (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *n
 }
 
 static void
-clean_dns_servers (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *nextevent)
+clean_dns_servers (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -939,9 +972,9 @@ clean_dns_servers (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 	for (i = 0; i < rdata->dns_servers->len; ) {
 		NMNDiscDNSServer *item = &g_array_index (rdata->dns_servers, NMNDiscDNSServer, i);
 
-		if (item->lifetime != G_MAXUINT32) {
-			guint64 expiry = (guint64) item->timestamp + item->lifetime;
-			guint64 refresh = (guint64) item->timestamp + item->lifetime / 2;
+		if (item->lifetime != NM_NDISC_INFINITY) {
+			gint32 expiry = get_expiry (item);
+			gint32 refresh;
 
 			if (now >= expiry) {
 				g_array_remove_index (rdata->dns_servers, i);
@@ -949,6 +982,7 @@ clean_dns_servers (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 				continue;
 			}
 
+			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
@@ -959,7 +993,7 @@ clean_dns_servers (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 }
 
 static void
-clean_dns_domains (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint32 *nextevent)
+clean_dns_domains (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nextevent)
 {
 	NMNDiscDataInternal *rdata;
 	guint i;
@@ -969,9 +1003,9 @@ clean_dns_domains (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 	for (i = 0; i < rdata->dns_domains->len; ) {
 		NMNDiscDNSDomain *item = &g_array_index (rdata->dns_domains, NMNDiscDNSDomain, i);
 
-		if (item->lifetime != G_MAXUINT32) {
-			guint64 expiry = (guint64) item->timestamp + item->lifetime;
-			guint64 refresh = (guint64) item->timestamp + item->lifetime / 2;
+		if (item->lifetime != NM_NDISC_INFINITY) {
+			gint32 expiry = get_expiry (item);
+			gint32 refresh;
 
 			if (now >= expiry) {
 				g_array_remove_index (rdata->dns_domains, i);
@@ -979,6 +1013,7 @@ clean_dns_domains (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 				continue;
 			}
 
+			refresh = get_expiry_half (item);
 			if (now >= refresh)
 				solicit_routers (ndisc);
 			else if (*nextevent > refresh)
@@ -991,12 +1026,11 @@ clean_dns_domains (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap *changed, guint
 static gboolean timeout_cb (gpointer user_data);
 
 static void
-check_timestamps (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap changed)
+check_timestamps (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 	/* Use a magic date in the distant future (~68 years) */
-	guint32 never = G_MAXINT32;
-	guint32 nextevent = never;
+	gint32 nextevent = G_MAXINT32;
 
 	nm_clear_g_source (&priv->timeout_id);
 
@@ -1009,10 +1043,11 @@ check_timestamps (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap changed)
 	if (changed)
 		_emit_config_change (ndisc, changed);
 
-	if (nextevent != never) {
-		g_return_if_fail (nextevent > now);
-		_LOGD ("scheduling next now/lifetime check: %u seconds",
-		       nextevent - now);
+	if (nextevent != G_MAXINT32) {
+		if (nextevent <= now)
+			g_return_if_reached ();
+		_LOGD ("scheduling next now/lifetime check: %d seconds",
+		       (int) (nextevent - now));
 		priv->timeout_id = g_timeout_add_seconds (nextevent - now, timeout_cb, ndisc);
 	}
 }
@@ -1028,7 +1063,7 @@ timeout_cb (gpointer user_data)
 }
 
 void
-nm_ndisc_ra_received (NMNDisc *ndisc, guint32 now, NMNDiscConfigMap changed)
+nm_ndisc_ra_received (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap changed)
 {
 	NMNDiscPrivate *priv = NM_NDISC_GET_PRIVATE (ndisc);
 
diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h
index 8b5f2bd5b1..17064b6883 100644
--- a/src/ndisc/nm-ndisc.h
+++ b/src/ndisc/nm-ndisc.h
@@ -62,6 +62,8 @@ typedef enum {
 	NM_NDISC_PREFERENCE_HIGH
 } NMNDiscPreference;
 
+#define NM_NDISC_INFINITY  G_MAXUINT32
+
 struct _NMNDiscGateway {
 	struct in6_addr address;
 	guint32 timestamp;