NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
kern/58927: itimer(9): overrun accounting is broken
>Number: 58927
>Category: kern
>Synopsis: itimer(9): overrun accounting is broken
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat Dec 21 17:15:00 +0000 2024
>Originator: Taylor R Campbell
>Release: current, 10
>Organization:
The TimeBSD Overrundation
>Environment:
>Description:
If you set a real-time periodic interval timer with timer_create(CLOCK_REALTIME) and timer_settime, and then the real-time clock is wound forward (e.g., via ntp adjustment), timer_getoverrun is supposed to return the number of missed events (up to DELAYTIMER_MAX, which we define to be 32).
But in an attempt to fix a arithmetic overflow, I broke what I thought was an arithmetic overflow check and was actually a clock-wound-forward check in kern_time.c itimer_callout:
Module Name: src
Committed By: riastradh
Date: Mon Jun 27 00:34:24 UTC 2022
Modified Files:
src/sys/kern: kern_time.c
Log Message:
setitimer(2): Avoid arithmetic overflow in periodic bookkeeping.
Reported-by: syzbot+93cef6090844ec304cde%syzkaller.appspotmail.com@localhost
https://mail-index.netbsd.org/source-changes/2022/06/27/msg139421.html
backwards = (timespeccmp(&it->it_time.it_value, &now, >));
- timespecadd(&it->it_time.it_value, &it->it_time.it_interval, &next);
+
+ /* Nonnegative interval guaranteed by itimerfix. */
+ KASSERT(it->it_time.it_interval.tv_sec >= 0);
+ KASSERT(it->it_time.it_interval.tv_nsec >= 0);
+
/* Handle the easy case of non-overflown timers first. */
- if (!backwards && timespeccmp(&next, &now, >)) {
+ if (!backwards &&
+ timespecaddok(&it->it_time.it_value, &it->it_time.it_interval)) {
+ timespecadd(&it->it_time.it_value, &it->it_time.it_interval,
+ &next);
it->it_time.it_value = next;
So none of the logic to count overruns ever triggers any more.
>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 += 1000;
printf("hack: %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);
}
Expected output: `overrun 32' or `overrun 1000' or something
Actual output: `overrun 0'.
>Fix:
Fix the logic in itimer_callout.
But first, need automatic tests for this arithmetic!
Home |
Main Index |
Thread Index |
Old Index