Hi,
Paul de Weerd mentioned off-list that the initial expiration for an
ITIMER_REAL timer is always at least one tick. I looked into it and
yes, this is the case, because the kernel rounds it_value up to one
tick if it is non-zero.
After thinking about it a bit I don't think we should do this
rounding. At least, not for the initial expiration.
Rounding the it_interval member of an itimerval value up to one tick
makes sense: if we don't round it up we might spin for a long time in
realitexpire() and itimerdecr() when we reload the timer.
However there is no such risk with the it_value member. We only use
it once and then it gets clobbered.
Currently the rounding is done in itimerfix(), which takes a timeval
pointer as argument. Given that itimerfix() is used nowhere else in
the kernel I think the easiest thing to do here is to rewrite
itimerfix() to take an itimerval pointer as argument and have it do
all input validation and normalization for setitimer(2) in one go:
- Validate it_value, return EINVAL if not.
- Validate it_interval, return EINVAL if not.
- Clear it_interval if it_value is unset.
- Round it_interval up if necessary.
The 100 million second upper bound for it_value and it_interval is
arbitrary and will probably change in the future, so I have isolated
that check from the others.
While we're changing the itimerfix() prototype we may as well pull it
out of sys/time.h. As I said before, it isn't used anywhere else.
OK?
Index: sys/time.h
===================================================================
RCS file: /cvs/src/sys/sys/time.h,v
retrieving revision 1.58
diff -u -p -r1.58 time.h
--- sys/time.h 13 Jan 2021 16:28:50 -0000 1.58
+++ sys/time.h 12 May 2021 17:06:30 -0000
@@ -307,7 +307,6 @@ struct proc;
int clock_gettime(struct proc *, clockid_t, struct timespec *);
void cancel_all_itimers(void);
-int itimerfix(struct timeval *);
int itimerdecr(struct itimerspec *, long);
int settime(const struct timespec *);
int ratecheck(struct timeval *, const struct timeval *);
Index: kern/kern_time.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.151
diff -u -p -r1.151 kern_time.c
--- kern/kern_time.c 23 Dec 2020 20:45:02 -0000 1.151
+++ kern/kern_time.c 12 May 2021 17:06:30 -0000
@@ -52,6 +52,8 @@
#include <dev/clock_subr.h>
+int itimerfix(struct itimerval *);
+
/*
* Time of day and interval timer support.
*
@@ -628,10 +630,9 @@ sys_setitimer(struct proc *p, void *v, r
error = copyin(SCARG(uap, itv), &aitv, sizeof(aitv));
if (error)
return error;
- if (itimerfix(&aitv.it_value) || itimerfix(&aitv.it_interval))
- return EINVAL;
- if (!timerisset(&aitv.it_value))
- timerclear(&aitv.it_interval);
+ error = itimerfix(&aitv);
+ if (error)
+ return error;
newitvp = &aitv;
}
if (SCARG(uap, oitv) != NULL) {
@@ -701,21 +702,34 @@ out:
}
/*
- * Check that a proposed value to load into the .it_value or
- * .it_interval part of an interval timer is acceptable.
+ * Check if the given setitimer(2) timer is valid. Clear it_interval
+ * if it_value is unset. Round it_interval up to the minimum interval
+ * if necessary.
*/
int
-itimerfix(struct timeval *tv)
+itimerfix(struct itimerval *itv)
{
+ struct timeval min_interval = { .tv_sec = 0, .tv_usec = tick };
- if (tv->tv_sec < 0 || tv->tv_sec > 100000000 ||
- tv->tv_usec < 0 || tv->tv_usec >= 1000000)
- return (EINVAL);
+ if (itv->it_value.tv_sec < 0 || !timerisvalid(&itv->it_value))
+ return EINVAL;
+ if (itv->it_value.tv_sec > 100000000)
+ return EINVAL;
- if (tv->tv_sec == 0 && tv->tv_usec != 0 && tv->tv_usec < tick)
- tv->tv_usec = tick;
+ if (itv->it_interval.tv_sec < 0 || !timerisvalid(&itv->it_interval))
+ return EINVAL;
+ if (itv->it_interval.tv_sec > 100000000)
+ return EINVAL;
- return (0);
+ if (!timerisset(&itv->it_value))
+ timerclear(&itv->it_interval);
+
+ if (timerisset(&itv->it_interval)) {
+ if (timercmp(&itv->it_interval, &min_interval, <))
+ itv->it_interval = min_interval;
+ }
+
+ return 0;
}
/*
