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
Unknown Object (File)
Fri, Oct 25, 2:30 AM
Unknown Object (File)
Oct 7 2024, 3:09 PM
Unknown Object (File)
Sep 23 2024, 11:20 PM
Unknown Object (File)
Sep 23 2024, 12:26 AM
Unknown Object (File)
Sep 22 2024, 4:54 PM
Unknown Object (File)
Sep 18 2024, 7:44 PM
Unknown Object (File)
Sep 17 2024, 2:50 AM
Unknown Object (File)
Sep 15 2024, 2:54 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
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 41852
Build 38740: arc lint + arc unit

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.