git: 121740e18515 - stable/13 - kqueue: don't arbitrarily restrict long-past values for NOTE_ABSTIME

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Wed, 06 Oct 2021 07:04:37 UTC
The branch stable/13 has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=121740e18515f56f54931c3d0dbbca620175c2a8

commit 121740e18515f56f54931c3d0dbbca620175c2a8
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2021-09-29 19:55:59 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2021-10-06 07:03:53 +0000

    kqueue: don't arbitrarily restrict long-past values for NOTE_ABSTIME
    
    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 9c999a259f00b35f0467acd351fea9157ed7e1e4)
    (cherry picked from commit 2f4dbe279f6b5eb87ec493d96f6943ffdb603ba0)
---
 sys/kern/kern_event.c              |  12 +++-
 tests/sys/kqueue/libkqueue/timer.c | 114 +++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 91895d341184..2e9773ab5701 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -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);
 }
 
@@ -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);
diff --git a/tests/sys/kqueue/libkqueue/timer.c b/tests/sys/kqueue/libkqueue/timer.c
index cb22887be276..330c22c62bc5 100644
--- a/tests/sys/kqueue/libkqueue/timer.c
+++ b/tests/sys/kqueue/libkqueue/timer.c
@@ -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)
 {
@@ -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();