kern/58926: itimer(9) integer overflow in overrun counting

[campbell+netbsd%mumble.net@localhost]

>Number:         58926
>Category:       kern
>Synopsis:       itimer(9) integer overflow in overrun counting
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Dec 21 16:50:01 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The TimeBSD Overrundation
>Environment:
>Description:
When a real-time interval timer is configured with timer_create(CLOCK_REALTIME) and timer_settime, and the real-time clock is wound forwards by a large amount (e.g., because of ntp adjustment), the calculation of the number of overruns to be returned by timer_getoverrun can overflow INT_MAX:

    839 	uint64_t last_val, next_val, interval, now_ns;
...
    882 			it->it_overruns += (now_ns - last_val) / interval;

https://nxr.netbsd.org/xref/src/sys/kern/kern_time.c?r=1.223#836

    109 	int	it_overruns;	/* Overruns currently accumulating */

https://nxr.netbsd.org/xref/src/sys/sys/timevar.h?r=1.51#109

Instead, since timer_getoverrun is supposed to saturate at DELAYTIMER_MAX from limits.h (https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/limits.h.html, https://pubs.opengroup.org/onlinepubs/9699919799//functions/timer_settime.html), we should do the same with this arithmetic here (or saturate at INT_MAX if we really want to keep more precision, or use a larger integer if we _really_ want to keep more precision, for some reason).
>How-To-Repeat:
1. set a periodic interval timer (setitimer, timer_settime, timerfd_settime)
2. wind the clock forward by a lot (settimeofday, clock_settime, ntp_adjtime, ...)

The following program _would_ demonstrate this, except it sends softclock callouts into a loop or something so it triggers a heartbeat panic for another reason:

#include <err.h>
#include <errno.h>
#include <limits.h>
#include <signal.h>
#include <stdio.h>
#include <time.h>
#include <unistd.h>

static const char *
showtime(struct timespec t)
{
	static char buf[1024];
	struct tm tm;
	size_t n;

	gmtime_r(&t.tv_sec, &tm);
	n = strftime(buf, sizeof(buf), "%Y-%m-%dT%H:%M:%S.", &tm);
	snprintf(buf + n, sizeof(buf) - n, "%09d", t.tv_nsec);

	return buf;
}

int
main(void)
{
	struct timespec mono0, real0, hack, mono1, delta, real1;
	sigset_t sigs, mask;
	siginfo_t siginfo;
	timer_t t;

	/*
	 * Block signals so we can use sigtimedwait(2) to wait for the
	 * first wakeup.
	 */
	if (sigemptyset(&sigs) == -1)
		err(1, "sigemptyset");
	if (sigaddset(&sigs, SIGALRM) == -1)
		err(1, "sigaddset");
	if (sigprocmask(SIG_BLOCK, &sigs, &mask) == -1)
		err(1, "sigprocmask");

	/*
	 * Create a periodic interval timer on the real-time clock
	 * starting at the next tick and repeating every second after
	 * that.
	 */
	const struct itimerspec it = {
		.it_value = {0, 1},
		.it_interval = {1, 0},
	};
	if (timer_create(CLOCK_REALTIME, NULL, &t) == -1)
		err(1, "timer_create");
	if (timer_settime(t, 0, &it, NULL) == -1)
		err(1, "timer_settime");

	/*
	 * Save the monotonic clock so, after we mess with the
	 * real-time clock, we can find how long we spent in this
	 * program to restore the real-time clock.
	 */
	if (clock_gettime(CLOCK_MONOTONIC, &mono0) == -1)
		err(1, "clock_gettime(CLOCK_MONOTONIC)");
	printf("mono0 %llu.%09d\n",
	    (unsigned long long)mono0.tv_sec, (int)mono0.tv_nsec);

	/*
	 * Advance the real-time clock by INT_MAX + 1 seconds.  This
	 * should cause the timer overrun counter to overflow.
	 */
	if (clock_gettime(CLOCK_REALTIME, &real0) == -1)
		err(1, "clock_gettime(CLOCK_REALTIME)");
	printf("real0 %s\n", showtime(real0));
	hack = real0;
	hack.tv_sec += (time_t)INT_MAX + 1;
	printf("hack clock to %s\n", showtime(hack));
	if (clock_settime(CLOCK_REALTIME, &hack) == -1)
		err(1, "clock_settime(CLOCK_REALTIME)");

	/*
	 * Wait up to two seconds for the timer to fire after an
	 * interval.  At this point it should have detected some
	 * overruns because we wound the clock forward.
	 *
	 * If anyting goes wrong, try to restore the real-time clock
	 * before reporting sigtimedwait(2) error.
	 */
	if (sigtimedwait(&sigs, &siginfo, &(const struct timespec){2, 0})
	    == -1) {
		int errno_save = errno;
		if (clock_gettime(CLOCK_MONOTONIC, &mono1) == -1)
			err(1, "clock_gettime");
		timespecsub(&mono1, &mono0, &delta);
		timespecadd(&real0, &delta, &real1);
		if (clock_settime(CLOCK_REALTIME, &real1) == -1)
			err(1, "clock_settime(CLOCK_REALTIME)");
		printf("mono1 %llu.%09d\n",
		    (unsigned long long)mono1.tv_sec, (int)mono1.tv_nsec);
		printf("delta %llu.%09d\n",
		    (unsigned long long)delta.tv_sec, (int)delta.tv_nsec);
		printf("real1 %s\n", showtime(real1));
		errno = errno_save;
		err(1, "sigtimedwait");
	}

	/*
	 * Restore the real-time clock by adding the time spent in this
	 * program so far (mono1 - mono0) to the earlier reading of the
	 * real-time clock (real0).
	 */
	if (clock_gettime(CLOCK_MONOTONIC, &mono1) == -1)
		err(1, "clock_gettime");
	timespecsub(&mono1, &mono0, &delta);
	timespecadd(&real0, &delta, &real1);
	if (clock_settime(CLOCK_REALTIME, &real1) == -1)
		err(1, "clock_settime(CLOCK_REALTIME)");
	printf("mono1 %llu.%09d\n",
	    (unsigned long long)mono1.tv_sec, (int)mono1.tv_nsec);
	printf("delta %llu.%09d\n",
	    (unsigned long long)delta.tv_sec, (int)delta.tv_nsec);
	printf("real1 %s\n", showtime(real1));

	/*
	 * Print the overrun count.  This should saturate at INT_MAX,
	 * and should never go negative.
	 */
	printf("overrun %d\n", timer_getoverrun(t));

	fflush(stdout);
	return ferror(stdout);
}

>Fix:
Saturate the arithmetic at INT_MAX.  (FreeBSD also delivers ERANGE in siginfo_t::si_errno.)