On Wed, May 19, 2021 at 10:32:55AM -0500, Scott Cheloha wrote: > On Wed, May 12, 2021 at 01:15:05PM -0500, Scott Cheloha wrote: > > > > [...] > > > > 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. > > > > [...] > > > > 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? > > Ping.
2 week bump. 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 27 May 2021 23:28:20 -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) input 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; } /* 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 27 May 2021 23:28:20 -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 *);