From b086535cb7f28a495a742c64ed4e212175f19860 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Oct 2018 13:05:51 +0200 Subject: 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. --- src/ndisc/nm-ndisc.c | 112 ++++++++++++++++++++++++--------------------------- src/ndisc/nm-ndisc.h | 3 ++ 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 { -- cgit v1.2.1