From owner-freebsd-hackers@FreeBSD.ORG Tue Oct 9 16:02:38 2012 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id BBCE2F25 for ; Tue, 9 Oct 2012 16:02:38 +0000 (UTC) (envelope-from freebsd@damnhippie.dyndns.org) Received: from duck.symmetricom.us (duck.symmetricom.us [206.168.13.214]) by mx1.freebsd.org (Postfix) with ESMTP id C5E898FC16 for ; Tue, 9 Oct 2012 16:02:31 +0000 (UTC) Received: from damnhippie.dyndns.org (daffy.symmetricom.us [206.168.13.218]) by duck.symmetricom.us (8.14.5/8.14.5) with ESMTP id q99G2OrY096571 for ; Tue, 9 Oct 2012 10:02:24 -0600 (MDT) (envelope-from freebsd@damnhippie.dyndns.org) Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id q99G2LCZ080554; Tue, 9 Oct 2012 10:02:21 -0600 (MDT) (envelope-from freebsd@damnhippie.dyndns.org) Subject: Re: time_t when used as timedelta From: Ian Lepore To: Erik Cederstrand In-Reply-To: <787F09EF-E3F7-467E-B023-B7846509D2A1@cederstrand.dk> References: <787F09EF-E3F7-467E-B023-B7846509D2A1@cederstrand.dk> Content-Type: text/plain; charset="us-ascii" Date: Tue, 09 Oct 2012 10:02:21 -0600 Message-ID: <1349798541.1123.6.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: FreeBSD Hackers X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Oct 2012 16:02:38 -0000 On Tue, 2012-10-09 at 17:35 +0200, Erik Cederstrand wrote: > Hi list, > > I'm looking at this possible divide-by zero in dhclient: http://scan.freebsd.your.org/freebsd-head/WORLD/2012-10-07-amd64/report-nBhqE2.html.gz#EndPath > > In this specific case, it's obvious from the intention of the code that ip->client->interval is always >0, but it's not obvious to me in the code. I could add an assert before the possible divide-by-zero: > > assert(ip->client->interval > 0); > > But looking at the code, I'm not sure it's very elegant. ip->client->interval is defined as time_t (see src/sbin/dhclient/dhcpd.h), which is a signed integer type, if I'm correct. However, some time_t members of struct client_state and struct client_config (see said header file) are assumed in the code to be positive and possibly non-null. Instead of plastering the code with asserts, is there something like an utime_t type? Or are there better ways to enforce the invariant? > It looks to me like the place where enforcement is really needed is in parse_lease_time() which should ensure at the very least that negative values never get through, and in some cases that zeroes don't sneak in from config files. If it were ensured that ip->client->config->backoff_cutoff could never be less than 1 (and it appears any value less than 1 would be insane), then the division by zero case could never happen. However, at least one of the config statements handled by parse_lease_time() allows a value of zero. Since nothing seems to ensure that backoff_cutoff is non-zero, it seems like a potential source of div-by-zero errors too, in that same function. -- Ian