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 _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
