Page MenuHomeFreeBSD

kqueue: don't arbitrarily restrict long-past values for NOTE_ABSTIME
ClosedPublic

Authored by kevans on Sep 29 2021, 8:27 PM.
Tags
None
Referenced Files
F95912729: D32230.diff
Mon, Sep 23, 12:26 AM
F95796288: D32230.diff
Sun, Sep 22, 4:54 PM
Unknown Object (File)
Wed, Sep 18, 7:44 PM
Unknown Object (File)
Tue, Sep 17, 2:50 AM
Unknown Object (File)
Sun, Sep 15, 2:54 PM
Unknown Object (File)
Sat, Sep 14, 9:32 PM
Unknown Object (File)
Mon, Sep 9, 8:48 PM
Unknown Object (File)
Mon, Sep 9, 8:47 PM
Subscribers

Details

Summary

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Would it be useful to assert to >= 0 in filt_timerstart()?

This revision is now accepted and ready to land.Sep 30 2021, 2:25 AM
In D32230#727342, @kib wrote:

Would it be useful to assert to >= 0 in filt_timerstart()?

I could see some ways to subtly break this, yeah. I've locally added:

diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c
index 1aab3679927..3cd7753d4f6 100644
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -815,9 +815,14 @@ filt_timerattach(struct knote *kn)
        sbintime_t to;
        int error;
 
+       to = -1;
        error = filt_timervalidate(kn, &to);
        if (error != 0)
                return (error);
+       KASSERT((kn->kn_flags & EV_ONESHOT) != 0 || to > 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);

It might be worth updating the man page to note this behaviour.

tests/sys/kqueue/libkqueue/timer.c
332

Extra newline.