From owner-svn-src-stable-11@freebsd.org Mon May 22 10:28:18 2017 Return-Path: Delivered-To: svn-src-stable-11@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 C17FCD77995; Mon, 22 May 2017 10:28:18 +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 82144165B; Mon, 22 May 2017 10:28:18 +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 v4MASHv5041438; Mon, 22 May 2017 10:28:17 GMT (envelope-from n_hibma@FreeBSD.org) Received: (from n_hibma@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v4MASHNe041436; Mon, 22 May 2017 10:28:17 GMT (envelope-from n_hibma@FreeBSD.org) Message-Id: <201705221028.v4MASHNe041436@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: n_hibma set sender to n_hibma@FreeBSD.org using -f From: Nick Hibma Date: Mon, 22 May 2017 10:28:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r318630 - stable/11/sbin/dhclient X-SVN-Group: stable-11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 May 2017 10:28:18 -0000 Author: n_hibma Date: Mon May 22 10:28:17 2017 New Revision: 318630 URL: https://svnweb.freebsd.org/changeset/base/318630 Log: 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 Modified: stable/11/sbin/dhclient/dhclient.c stable/11/sbin/dhclient/options.c Modified: stable/11/sbin/dhclient/dhclient.c ============================================================================== --- stable/11/sbin/dhclient/dhclient.c Mon May 22 08:20:50 2017 (r318629) +++ stable/11/sbin/dhclient/dhclient.c Mon May 22 10:28:17 2017 (r318630) @@ -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); } Modified: stable/11/sbin/dhclient/options.c ============================================================================== --- stable/11/sbin/dhclient/options.c Mon May 22 08:20:50 2017 (r318629) +++ stable/11/sbin/dhclient/options.c Mon May 22 10:28:17 2017 (r318630) @@ -783,7 +783,7 @@ pretty_print_option(unsigned int code, u dp += 4; break; case 'L': - opcount = snprintf(op, opleft, "%ld", + opcount = snprintf(op, opleft, "%lu", (unsigned long)getULong(dp)); if (opcount >= opleft || opcount == -1) goto toobig; @@ -799,7 +799,7 @@ pretty_print_option(unsigned int code, u dp += 2; break; case 'S': - opcount = snprintf(op, opleft, "%d", + opcount = snprintf(op, opleft, "%u", getUShort(dp)); if (opcount >= opleft || opcount == -1) goto toobig;