NetBSD Problem Report #58926

From www@netbsd.org  Sat Dec 21 16:46:23 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E44391A923A
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 21 Dec 2024 16:46:23 +0000 (UTC)
Message-Id: <20241221164622.402E91A923B@mollari.NetBSD.org>
Date: Sat, 21 Dec 2024 16:46:22 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: itimer(9) integer overflow in overrun counting
X-Send-Pr-Version: www-1.0

>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
>Last-Modified:  Sun Dec 22 23:20:04 +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.)

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58926 CVS commit: src/lib/libc/gen
Date: Sun, 22 Dec 2024 23:17:26 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Dec 22 23:17:26 UTC 2024

 Modified Files:
 	src/lib/libc/gen: sysconf.c

 Log Message:
 sysconf(3): KNF

 No functional change intended.

 Prompted by exposing DELAYTIMER_MAX in preparation for:

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


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.45 src/lib/libc/gen/sysconf.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58926 CVS commit: src/sys
Date: Sun, 22 Dec 2024 23:18:30 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Dec 22 23:18:29 UTC 2024

 Modified Files:
 	src/sys/kern: kern_time.c
 	src/sys/sys: syslimits.h

 Log Message:
 limits.h: Define DELAYTIMER_MAX.

 This is the maximum value of timer_getoverrun(), and was introduced
 in IEEE Std 1003.1b-1993.

 Prompted by:

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


 To generate a diff of this commit:
 cvs rdiff -u -r1.225 -r1.226 src/sys/kern/kern_time.c
 cvs rdiff -u -r1.28 -r1.29 src/sys/sys/syslimits.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search