summaryrefslogtreecommitdiffstats
path: root/sbin/dhclient/dhclient.c
diff options
context:
space:
mode:
authorn_hibma <n_hibma@FreeBSD.org>2017-05-22 10:28:17 +0000
committern_hibma <n_hibma@FreeBSD.org>2017-05-22 10:28:17 +0000
commit1f2961e6aa280c022a8e33b13e4364467dd0b009 (patch)
treef2b66c2c879c11e56e22c4ae6934e8abcfcda534 /sbin/dhclient/dhclient.c
parent23dcad981553a6eed093a676a47643e37b8de63b (diff)
downloadFreeBSD-src-1f2961e6aa280c022a8e33b13e4364467dd0b009.zip
FreeBSD-src-1f2961e6aa280c022a8e33b13e4364467dd0b009.tar.gz
MFC:
------------------------------------------------------------------------ r317923 | n_hibma | 2017-05-07 23:11:28 +0200 (Sun, 07 May 2017) | 8 lines Fix the output of very large rebind, renew and lease time options in lease file. Some routers set very large values for rebind time (Netgear) and these are erroneously reported as negative in the leasefile. This was due to a wrong printf format specification of %ld for an unsigned long on 32-bit platforms. ------------------------------------------------------------------------ r317915 | n_hibma | 2017-05-07 21:59:37 +0200 (Sun, 07 May 2017) | 16 lines Fix handling of large DHCP expiry values. They would overflow a signed 32-bit time_t on 32 bit architectures. This was taken care of, but a compiler optimisation makes this behave erratically. This could be resolved by adding a -fwrapv flag, but instead we can check the value before adding the current timestamp to it. In the lease file values are still wrong though: option dhcp-rebinding-time -644245096; PR: 218980
Diffstat (limited to 'sbin/dhclient/dhclient.c')
-rw-r--r--sbin/dhclient/dhclient.c42
1 files changed, 22 insertions, 20 deletions
diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index 2a43082..b68f636 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -107,7 +107,11 @@ struct pidfh *pidfile;
*/
#define ASSERT_STATE(state_is, state_shouldbe) {}
-#define TIME_MAX 2147483647
+/*
+ * We need to check that the expiry, renewal and rebind times are not beyond
+ * the end of time (~2038 when a 32-bit time_t is being used).
+ */
+#define TIME_MAX ((((time_t) 1 << (sizeof(time_t) * CHAR_BIT - 2)) - 1) * 2 + 1)
int log_priority;
int no_daemon;
@@ -762,15 +766,17 @@ dhcpack(struct packet *packet)
else
ip->client->new->expiry = default_lease_time;
/* A number that looks negative here is really just very large,
- because the lease expiry offset is unsigned. */
- if (ip->client->new->expiry < 0)
- ip->client->new->expiry = TIME_MAX;
+ because the lease expiry offset is unsigned. Also make sure that
+ the addition of cur_time below does not overflow (a 32 bit) time_t. */
+ if (ip->client->new->expiry < 0 ||
+ ip->client->new->expiry > TIME_MAX - cur_time)
+ ip->client->new->expiry = TIME_MAX - cur_time;
/* XXX should be fixed by resetting the client state */
if (ip->client->new->expiry < 60)
ip->client->new->expiry = 60;
/* Unless overridden in the config, take the server-provided renewal
- * time if there is one; otherwise figure it out according to the spec.
+ * time if there is one. Otherwise figure it out according to the spec.
* Also make sure the renewal time does not exceed the expiry time.
*/
if (ip->client->config->default_actions[DHO_DHCP_RENEWAL_TIME] ==
@@ -782,7 +788,8 @@ dhcpack(struct packet *packet)
ip->client->new->options[DHO_DHCP_RENEWAL_TIME].data);
else
ip->client->new->renewal = ip->client->new->expiry / 2;
- if (ip->client->new->renewal > ip->client->new->expiry / 2)
+ if (ip->client->new->renewal < 0 ||
+ ip->client->new->renewal > ip->client->new->expiry / 2)
ip->client->new->renewal = ip->client->new->expiry / 2;
/* Same deal with the rebind time. */
@@ -794,20 +801,15 @@ dhcpack(struct packet *packet)
ip->client->new->rebind = getULong(
ip->client->new->options[DHO_DHCP_REBINDING_TIME].data);
else
- ip->client->new->rebind = ip->client->new->renewal * 7 / 4;
- if (ip->client->new->rebind > ip->client->new->renewal * 7 / 4)
- ip->client->new->rebind = ip->client->new->renewal * 7 / 4;
-
- ip->client->new->expiry += cur_time;
- /* Lease lengths can never be negative. */
- if (ip->client->new->expiry < cur_time)
- ip->client->new->expiry = TIME_MAX;
- ip->client->new->renewal += cur_time;
- if (ip->client->new->renewal < cur_time)
- ip->client->new->renewal = TIME_MAX;
- ip->client->new->rebind += cur_time;
- if (ip->client->new->rebind < cur_time)
- ip->client->new->rebind = TIME_MAX;
+ ip->client->new->rebind = ip->client->new->renewal / 4 * 7;
+ if (ip->client->new->rebind < 0 ||
+ ip->client->new->rebind > ip->client->new->renewal / 4 * 7)
+ ip->client->new->rebind = ip->client->new->renewal / 4 * 7;
+
+ /* Convert the time offsets into seconds-since-the-epoch */
+ ip->client->new->expiry += cur_time;
+ ip->client->new->renewal += cur_time;
+ ip->client->new->rebind += cur_time;
bind_lease(ip);
}
OpenPOWER on IntegriCloud