Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 09 Oct 2012 10:02:21 -0600
From:      Ian Lepore <freebsd@damnhippie.dyndns.org>
To:        Erik Cederstrand <erik@cederstrand.dk>
Cc:        FreeBSD Hackers <hackers@freebsd.org>
Subject:   Re: time_t when used as timedelta
Message-ID:  <1349798541.1123.6.camel@revolution.hippie.lan>
In-Reply-To: <787F09EF-E3F7-467E-B023-B7846509D2A1@cederstrand.dk>
References:  <787F09EF-E3F7-467E-B023-B7846509D2A1@cederstrand.dk>

next in thread | previous in thread | raw e-mail | index | archive | help
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





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1349798541.1123.6.camel>