summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Haller <thaller@redhat.com>2018-10-12 13:05:51 +0200
committerThomas Haller <thaller@redhat.com>2018-10-13 17:11:52 +0200
commitb086535cb7f28a495a742c64ed4e212175f19860 (patch)
treeb3350c32bf1f4b33ded0976b8e10bef53b37a954
parent9d0a138ef05544954de49a8056f47e34c6bf3c5a (diff)
downloadNetworkManager-b086535cb7f28a495a742c64ed4e212175f19860.tar.gz
ndisc: handle integer overflows better for lifetime handling
we use get_expiry() to compare two lifetimes. Note, that previously, it would correctly truncate the calculated expiry at G_MAXINT32-1. However, that means, that two different lifetimes that both lie more than 68 years in the future would compare equal. Fix that, but extending the range to int64, so that no overflow can happen.
-rw-r--r--src/ndisc/nm-ndisc.c112
-rw-r--r--src/ndisc/nm-ndisc.h3
2 files changed, 56 insertions, 59 deletions
diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c
index 0c2dbf8cdd..6c619ad3ce 100644
--- a/src/ndisc/nm-ndisc.c
+++ b/src/ndisc/nm-ndisc.c
@@ -124,53 +124,66 @@ _preference_to_priority (NMIcmpv6RouterPref pref)
/*****************************************************************************/
-static gint32
+/* we rely on the fact, that _EXPIRY_INFINITY > any other valid gint64 timestamps. */
+#define _EXPIRY_INFINITY G_MAXINT64
+
+static gint64
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);
+ return _EXPIRY_INFINITY;
+ return ((gint64) timestamp) + ((gint64) lifetime);
}
#define get_expiry(item) \
({ \
typeof (item) _item = (item); \
nm_assert (_item); \
- get_expiry_time ((_item->timestamp), (_item->lifetime)); \
+ 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); \
+ (_item->lifetime == NM_NDISC_INFINITY) \
+ ? _EXPIRY_INFINITY \
+ : get_expiry_time (_item->timestamp, _item->lifetime / 2); \
})
#define get_expiry_preferred(item) \
({ \
typeof (item) _item = (item); \
nm_assert (_item); \
- get_expiry_time ((_item->timestamp), (_item->preferred)); \
+ get_expiry_time (_item->timestamp, _item->preferred); \
})
+static gboolean
+expiry_next (gint32 now_s, gint64 expiry_timestamp, gint32 *nextevent)
+{
+ gint32 e;
+
+ if (expiry_timestamp == _EXPIRY_INFINITY)
+ return TRUE;
+ e = MIN (expiry_timestamp, ((gint64) (G_MAXINT32 - 1)));
+ if (now_s >= e)
+ return FALSE;
+ if (nextevent) {
+ if (*nextevent > e)
+ *nextevent = e;
+ }
+ return TRUE;
+}
+
static const char *
-_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint32 expiry_time)
+_get_exp (char *buf, gsize buf_size, gint64 now_ns, gint64 expiry_time)
{
int l;
- if (expiry_time == G_MAXINT32)
+ if (expiry_time == _EXPIRY_INFINITY)
return "permanent";
l = g_snprintf (buf, buf_size,
"%.4f",
@@ -1029,17 +1042,12 @@ clean_gateways (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *n
for (i = 0; i < rdata->gateways->len; ) {
NMNDiscGateway *item = &g_array_index (rdata->gateways, NMNDiscGateway, i);
- if (item->lifetime != NM_NDISC_INFINITY) {
- gint32 expiry = get_expiry (item);
-
- if (now >= expiry) {
- g_array_remove_index (rdata->gateways, i);
- *changed |= NM_NDISC_CONFIG_GATEWAYS;
- continue;
- }
- if (*nextevent > expiry)
- *nextevent = expiry;
+ if (!expiry_next (now, get_expiry (item), nextevent)) {
+ g_array_remove_index (rdata->gateways, i);
+ *changed |= NM_NDISC_CONFIG_GATEWAYS;
+ continue;
}
+
i++;
}
@@ -1057,17 +1065,12 @@ clean_addresses (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *
for (i = 0; i < rdata->addresses->len; ) {
const NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i);
- if (item->lifetime != NM_NDISC_INFINITY) {
- gint32 expiry = get_expiry (item);
-
- if (now >= expiry) {
- g_array_remove_index (rdata->addresses, i);
- *changed |= NM_NDISC_CONFIG_ADDRESSES;
- continue;
- }
- if (*nextevent > expiry)
- *nextevent = expiry;
+ if (!expiry_next (now, get_expiry (item), nextevent)) {
+ g_array_remove_index (rdata->addresses, i);
+ *changed |= NM_NDISC_CONFIG_ADDRESSES;
+ continue;
}
+
i++;
}
}
@@ -1083,17 +1086,12 @@ clean_routes (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32 *nex
for (i = 0; i < rdata->routes->len; ) {
NMNDiscRoute *item = &g_array_index (rdata->routes, NMNDiscRoute, i);
- if (item->lifetime != NM_NDISC_INFINITY) {
- gint32 expiry = get_expiry (item);
-
- if (now >= expiry) {
- g_array_remove_index (rdata->routes, i);
- *changed |= NM_NDISC_CONFIG_ROUTES;
- continue;
- }
- if (*nextevent > expiry)
- *nextevent = expiry;
+ if (!expiry_next (now, get_expiry (item), nextevent)) {
+ g_array_remove_index (rdata->routes, i);
+ *changed |= NM_NDISC_CONFIG_ROUTES;
+ continue;
}
+
i++;
}
}
@@ -1108,18 +1106,16 @@ clean_dns_servers (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
for (i = 0; i < rdata->dns_servers->len; ) {
NMNDiscDNSServer *item = &g_array_index (rdata->dns_servers, NMNDiscDNSServer, i);
+ gint64 refresh;
- if (item->lifetime != NM_NDISC_INFINITY) {
- gint32 expiry = get_expiry (item);
- gint32 refresh;
-
- if (now >= expiry) {
+ refresh = get_expiry_half (item);
+ if (refresh != _EXPIRY_INFINITY) {
+ if (!expiry_next (now, get_expiry (item), NULL)) {
g_array_remove_index (rdata->dns_servers, i);
*changed |= NM_NDISC_CONFIG_DNS_SERVERS;
continue;
}
- refresh = get_expiry_half (item);
if (now >= refresh)
solicit_routers (ndisc);
else if (*nextevent > refresh)
@@ -1139,18 +1135,16 @@ clean_dns_domains (NMNDisc *ndisc, gint32 now, NMNDiscConfigMap *changed, gint32
for (i = 0; i < rdata->dns_domains->len; ) {
NMNDiscDNSDomain *item = &g_array_index (rdata->dns_domains, NMNDiscDNSDomain, i);
+ gint64 refresh;
- if (item->lifetime != NM_NDISC_INFINITY) {
- gint32 expiry = get_expiry (item);
- gint32 refresh;
-
- if (now >= expiry) {
+ refresh = get_expiry_half (item);
+ if (refresh != _EXPIRY_INFINITY) {
+ if (!expiry_next (now, get_expiry (item), NULL)) {
g_array_remove_index (rdata->dns_domains, i);
*changed |= NM_NDISC_CONFIG_DNS_DOMAINS;
continue;
}
- refresh = get_expiry_half (item);
if (now >= refresh)
solicit_routers (ndisc);
else if (*nextevent > refresh)
diff --git a/src/ndisc/nm-ndisc.h b/src/ndisc/nm-ndisc.h
index fdc5615f54..73eef368ed 100644
--- a/src/ndisc/nm-ndisc.h
+++ b/src/ndisc/nm-ndisc.h
@@ -58,6 +58,9 @@ typedef enum {
NM_NDISC_DHCP_LEVEL_MANAGED
} NMNDiscDHCPLevel;
+/* we rely on the fact that NM_NDISC_INFINITY is the largest possible
+ * time duration (G_MAXUINT32) and that the range of finite values
+ * goes from 0 to G_MAXUINT32-1. */
#define NM_NDISC_INFINITY G_MAXUINT32
struct _NMNDiscGateway {