Bug #22033 for dhcp-public: dhcp-4.2.0 uses int for handling time values

On 11/25/2010 12:38 AM, Shawn Routhier via RT wrote: > Thank you for the bug report. We've chose a different solution in order to > limit what gets passed into the isc_interval_set call. I'm attaching a copy of > the diff to the ticket. I would appreciate it if you can verify the fix > (assuming the testbed is still available). Hi, I verify that the diff you sent me fixes the problem. Jiri > Cut here for diff > > <snip>

---

Hi Shawn, see my small remark lower in the code. Jiri On 11/25/2010 12:38 AM, Shawn Routhier via RT wrote: > Thank you for the bug report. We've chose a different solution in order to > limit what gets passed into the isc_interval_set call. I'm attaching a copy of > the diff to the ticket. I would appreciate it if you can verify the fix > (assuming the testbed is still available). > > Cut here for diff > > --- DHCP/RELNOTES:1.405 Thu Nov 18 19:47:36 2010 > +++ DHCP/RELNOTES Wed Nov 24 22:25:02 2010 > @@ -144,6 +144,9 @@ > a setsockopt() call. The signal is already being ignored as part > of the ISC library. [ISC-Bugs #22269] > > +- Limit the timeout period allowed in the dispatch code to 2^^32-1 seconds. > + Thanks to a report from Jiri Popelka at Red Hat. > + [ISC-Bugs #22033], [Red Hat Bug #628258] > > Changes since 4.2.0b2 > > Index: DHCP/common/dispatch.c > diff -u DHCP/common/dispatch.c:1.75 DHCP/common/dispatch.c:1.75.46.1 > --- DHCP/common/dispatch.c:1.75 Tue May 25 00:22:29 2010 > +++ DHCP/common/dispatch.c Wed Nov 24 22:25:02 2010 > @@ -174,6 +174,7 @@ > > /* maximum value for usec */ > #define USEC_MAX 1000000 > +#define DHCP_SEC_MAX 0xFFFFFFFF > > void add_timeout (when, where, what, ref, unref) > struct timeval *when; > @@ -185,7 +186,8 @@ > struct timeout *t, *q; > int usereset = 0; > isc_result_t status; > - int sec, usec; > + int64_t sec; > + int usec; > isc_interval_t interval; > isc_time_t expires; > > @@ -227,9 +229,49 @@ > q->what = what; > } > > - /* We don't really need this, but keep it for now */ > - q->when.tv_sec = when->tv_sec; > - q->when.tv_usec = when->tv_usec; > + /* > + * The value passed in is a time from an epoch but we need a relative > + * time so we need to do some math to try and recover the period. > + * This is complicated by the fact that not all of the calls cared > + * about the usec value, if it's zero we assume the caller didn't care. > + * > + * The ISC timer library doesn't seem to like negative values > + * and can't accept any values above 4G-1 seconds so we limit > + * the values to 0<= value< 4G-1. We do it before > + * checking the trace option so that both the trace code and > + * the working code use the same values. > + */ > + > + sec = when->tv_sec - cur_tv.tv_sec; > + usec = when->tv_usec - cur_tv.tv_usec; > + > + if ((when->tv_usec != 0)&& (usec< 0)) { > + sec--; > + usec += USEC_MAX; > + } > + > + if (sec< 0) { > + sec = 0; > + usec = 0; > + } else if (sec> DHCP_SEC_MAX) { > + log_error("Timeout requested too large %lld " > + "reducing to 2^^32-1", sec); > + sec = DHCP_SEC_MAX; > + usec = 0; > + } > + else if (usec< 0) { > + usec = 0; > + } else if (usec>= USEC_MAX) { > + usec = USEC_MAX - 1; > + } > + > + /* > + * This is necessary for the tracing code but we put it > + * here in case we want to compare timing information > + * for some reason, like debugging. > + */ > + q->when.tv_sec = cur_tv.tv_sec + (sec& DHCP_SEC_MAX); > + q->when.tv_usec = usec; > > #if defined (TRACING) > if (trace_playback()) { > @@ -279,38 +321,7 @@ > q->next = timeouts; > timeouts = q; > > - /* > - * Set up the interval values - The previous timers allowed > - * negative values to be set, the ISC timer library doesn't like > - * that so we make any negative values 0 which sould amount to > - * the same thing. > - */ > - > - /* > - * The value passed in is a time from an epoch but we need a relative > - * time so we need to do some math to try and recover the period. > - * This is complicated by the fact that not all of the calls cared > - * about the usec value, if it's zero we assume the caller didn't care. > - */ > - > - sec = when->tv_sec - cur_tv.tv_sec; > - usec = when->tv_usec - cur_tv.tv_usec; > - > - if ((when->tv_usec != 0)&& (usec< 0)) { > - sec--; > - usec += USEC_MAX; > - } > - > - if (sec< 0) { > - sec = 0; > - usec = 0; > - } else if (usec< 0) { > - usec = 0; > - } else if (usec>= USEC_MAX) { > - usec = USEC_MAX - 1; > - } > - > - isc_interval_set(&interval, sec, usec * 1000); > + isc_interval_set(&interval, sec& 0xFFFFFFFF, usec * 1000); Probably should be + isc_interval_set(&interval, sec& DHCP_SEC_MAX, usec * 1000); instead of + isc_interval_set(&interval, sec& 0xFFFFFFFF, usec * 1000); > status = isc_time_nowplusinterval(&expires,&interval); > if (status != ISC_R_SUCCESS) { > /* >

---