From owner-svn-src-all@freebsd.org Sun May 7 19:59:39 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4AC5ED624E4; Sun, 7 May 2017 19:59:39 +0000 (UTC) (envelope-from n_hibma@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0A8E212A7; Sun, 7 May 2017 19:59:38 +0000 (UTC) (envelope-from n_hibma@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v47JxbqQ013420; Sun, 7 May 2017 19:59:37 GMT (envelope-from n_hibma@FreeBSD.org) Received: (from n_hibma@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v47Jxbid013419; Sun, 7 May 2017 19:59:37 GMT (envelope-from n_hibma@FreeBSD.org) Message-Id: <201705071959.v47Jxbid013419@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: n_hibma set sender to n_hibma@FreeBSD.org using -f From: Nick Hibma Date: Sun, 7 May 2017 19:59:37 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r317915 - head/sbin/dhclient X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 May 2017 19:59:39 -0000 Author: n_hibma Date: Sun May 7 19:59:37 2017 New Revision: 317915 URL: https://svnweb.freebsd.org/changeset/base/317915 Log: 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 Reported by: Bob Eager MFC after: 2 weeks Modified: head/sbin/dhclient/dhclient.c Modified: head/sbin/dhclient/dhclient.c ============================================================================== --- head/sbin/dhclient/dhclient.c Sun May 7 19:57:45 2017 (r317914) +++ head/sbin/dhclient/dhclient.c Sun May 7 19:59:37 2017 (r317915) @@ -108,7 +108,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; @@ -766,15 +770,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] == @@ -786,7 +792,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. */ @@ -798,20 +805,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); }