Skip to content

Commit

Permalink
kqueue: don't arbitrarily restrict long-past values for NOTE_ABSTIME
Browse files Browse the repository at this point in the history
NOTE_ABSTIME values are converted to values relative to boottime in
filt_timervalidate(), and negative values are currently rejected.  We
don't reject times in the past in general, so clamp this up to 0 as
needed such that the timer fires immediately rather than imposing what
looks like an arbitrary restriction.

Another possible scenario is that the system clock had to be adjusted
by ~minutes or ~hours and we have less than that in terms of uptime,
making a reasonable short-timeout suddenly invalid. Firing it is still
a valid choice in this scenario so that applications can at least
expect a consistent behavior.

(cherry picked from commit 9c999a2)
(cherry picked from commit 2f4dbe2)
  • Loading branch information
kevans91 committed Oct 6, 2021
1 parent 06248c8 commit 121740e
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 3 deletions.
12 changes: 9 additions & 3 deletions sys/kern/kern_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -798,13 +798,13 @@ filt_timervalidate(struct knote *kn, sbintime_t *to)
return (EINVAL);

*to = timer2sbintime(kn->kn_sdata, kn->kn_sfflags);
if (*to < 0)
return (EINVAL);
if ((kn->kn_sfflags & NOTE_ABSTIME) != 0) {
getboottimebin(&bt);
sbt = bttosbt(bt);
*to -= sbt;
*to = MAX(0, *to - sbt);
}
if (*to < 0)
return (EINVAL);
return (0);
}

Expand All @@ -815,9 +815,15 @@ filt_timerattach(struct knote *kn)
sbintime_t to;
int error;

to = -1;
error = filt_timervalidate(kn, &to);
if (error != 0)
return (error);
KASSERT(to > 0 || (kn->kn_flags & EV_ONESHOT) != 0 ||
(kn->kn_sfflags & NOTE_ABSTIME) != 0,
("%s: periodic timer has a calculated zero timeout", __func__));
KASSERT(to >= 0,
("%s: timer has a calculated negative timeout", __func__));

if (atomic_fetchadd_int(&kq_ncallouts, 1) + 1 > kq_calloutmax) {
atomic_subtract_int(&kq_ncallouts, 1);
Expand Down
114 changes: 114 additions & 0 deletions tests/sys/kqueue/libkqueue/timer.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,117 @@ test_abstime(void)
success();
}

static void
test_abstime_epoch(void)
{
const char *test_id = "kevent(EVFILT_TIMER (EPOCH), NOTE_ABSTIME)";
struct kevent kev;

test_begin(test_id);

test_no_kevents();

EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD, NOTE_ABSTIME, 0,
NULL);
if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0)
err(1, "%s", test_id);

/* Retrieve the event */
kev.flags = EV_ADD;
kev.data = 1;
kev.fflags = 0;
kevent_cmp(&kev, kevent_get(kqfd));

/* Delete the event */
kev.flags = EV_DELETE;
if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0)
err(1, "%s", test_id);

success();
}

static void
test_abstime_preboot(void)
{
const char *test_id = "kevent(EVFILT_TIMER (PREBOOT), EV_ONESHOT, NOTE_ABSTIME)";
struct kevent kev;
struct timespec btp;
uint64_t end, start, stop;

test_begin(test_id);

test_no_kevents();

/*
* We'll expire it at just before system boot (roughly) with the hope that
* we'll get an ~immediate expiration, just as we do for any value specified
* between system boot and now.
*/
start = now();
if (clock_gettime(CLOCK_BOOTTIME, &btp) != 0)
err(1, "%s", test_id);

end = start - SEC_TO_US(btp.tv_sec + 1);
EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT,
NOTE_ABSTIME | NOTE_USECONDS, end, NULL);
if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0)
err(1, "%s", test_id);

/* Retrieve the event */
kev.flags = EV_ADD | EV_ONESHOT;
kev.data = 1;
kev.fflags = 0;
kevent_cmp(&kev, kevent_get(kqfd));

stop = now();
if (stop < end)
err(1, "too early %jd %jd", (intmax_t)stop, (intmax_t)end);
/* Check if the event occurs again */
sleep(3);
test_no_kevents();

success();
}

static void
test_abstime_postboot(void)
{
const char *test_id = "kevent(EVFILT_TIMER (POSTBOOT), EV_ONESHOT, NOTE_ABSTIME)";
struct kevent kev;
uint64_t end, start, stop;
const int timeout_sec = 1;

test_begin(test_id);

test_no_kevents();

/*
* Set a timer for 1 second ago, it should fire immediately rather than
* being rejected.
*/
start = now();
end = start - SEC_TO_US(timeout_sec);
EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT,
NOTE_ABSTIME | NOTE_USECONDS, end, NULL);
if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0)
err(1, "%s", test_id);

/* Retrieve the event */
kev.flags = EV_ADD | EV_ONESHOT;
kev.data = 1;
kev.fflags = 0;
kevent_cmp(&kev, kevent_get(kqfd));

stop = now();
if (stop < end)
err(1, "too early %jd %jd", (intmax_t)stop, (intmax_t)end);
/* Check if the event occurs again */
sleep(3);
test_no_kevents();

success();
}

static void
test_update(void)
{
Expand Down Expand Up @@ -517,6 +628,9 @@ test_evfilt_timer(void)
test_oneshot();
test_periodic();
test_abstime();
test_abstime_epoch();
test_abstime_preboot();
test_abstime_postboot();
test_update();
test_update_equal();
test_update_expired();
Expand Down

0 comments on commit 121740e

Please sign in to comment.