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.
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 19 May 2021 15:32:26 -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 19 May 2021 15:32:26 -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 *);
