[Dnsmasq-discuss] [PATCH] dnsmasq_time: avoid signed integer overflow when HAVE_BROKEN_RTC

[Simon Kelley]

Nice catch, and nice patch.

Patch applied.


Cheers,

Simon.


On 28/09/2021 01:44, Matt Whitlock wrote:
> The dnsmasq_time() function, in the case of HAVE_BROKEN_RTC, was calling
> times() to read the number of ticks "elapsed since an arbitrary point in
> the past" and then dividing that by sysconf(_SC_CLK_TCK) to compute the
> number of seconds elapsed since that arbitrary instant. This works fine
> until the number of ticks exceeds 2^31, beyond which time the function
> would begin erroneously returning negative times. On my system this
> happens after approximately 248 days of uptime. A symptom is that
> dnsmasq no longer populates the resolver cache with DHCP-derived names
> at startup, as the inserted cache entries immediately expire due to
> having negative expiration times that cause is_expired() to return true
> when called with now==0.
> 
> This commit replaces the archaic implementation of dnsmasq_time() with a
> call to the POSIX-standardized clock_gettime(CLOCK_MONOTONIC), thereby
> eliminating the need to convert manually from ticks to seconds. The new
> implementation will yield correct results until the system uptime
> exceeds approximately 68 years.
> 
> Signed-off-by: Matt Whitlock <dnsmasq at mattwhitlock.name>
> ---
>  src/util.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/src/util.c b/src/util.c
> index 1425764..b2179d0 100644
> --- a/src/util.c
> +++ b/src/util.c
> @@ -408,13 +408,12 @@ int hostname_issubdomain(char *a, char *b)
>  time_t dnsmasq_time(void)
>  {
>  #ifdef HAVE_BROKEN_RTC
> -  struct tms dummy;
> -  static long tps = 0;
> +  struct timespec ts;
>  
> -  if (tps == 0)
> -    tps = sysconf(_SC_CLK_TCK);
> +  if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0)
> +    die(_("cannot read monotonic clock: %s"), NULL, EC_MISC);
>  
> -  return (time_t)(times(&dummy)/tps);
> +  return ts.tv_sec;
>  #else
>    return time(NULL);
>  #endif
>