setitimer(2): consolidate copyin(9), input validation, input conversion

For what are probably historical reasons, setitimer(2) does not
validate its input (itv) immediately after copyin(9).  Instead, it
waits until after (possibly) performing a getitimer(2) to copy out the
state of the timer.

Consolidating copyin(9), input validation, and input conversion into a
single block before the getitimer(2) operation makes setitimer(2)
itself easier to read.  It will also simplify merging the critical
sections of setitimer(2) and getitimer(2) in a subsequent patch.

This changes setitimer(2)'s behavior in the EINVAL case.  Currently,
if your input (itv) is invalid, we return EINVAL *after* modifying the
output (olditv).  With the patch we will now return EINVAL *before*
modifying the output.  However, any code dependent upon this behavior
is broken: the contents of olditv are undefined in all setitimer(2)
error cases.

ok millert@

1 files changed, 10 insertions, 8 deletions

diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 73daea85537..aef1375ec10 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_time.c,v 1.136 2020/08/11 15:41:50 cheloha Exp $	*/
+/*	$OpenBSD: kern_time.c,v 1.137 2020/08/11 18:29:58 cheloha Exp $	*/
 /*	$NetBSD: kern_time.c,v 1.20 1996/02/18 11:57:06 fvdl Exp $	*/
 
 /*
@@ -580,9 +580,15 @@ sys_setitimer(struct proc *p, void *v, register_t *retval)
 	if (which < ITIMER_REAL || which > ITIMER_PROF)
 		return (EINVAL);
 	itvp = SCARG(uap, itv);
-	if (itvp && (error = copyin((void *)itvp, (void *)&aitv,
-	    sizeof(struct itimerval))))
-		return (error);
+	if (itvp) {
+		error = copyin(itvp, &aitv, sizeof(struct itimerval));
+		if (error)
+			return (error);
+		if (itimerfix(&aitv.it_value) || itimerfix(&aitv.it_interval))
+			return (EINVAL);
+		TIMEVAL_TO_TIMESPEC(&aitv.it_value, &aits.it_value);
+		TIMEVAL_TO_TIMESPEC(&aitv.it_interval, &aits.it_interval);
+	}
 	if (oitv != NULL) {
 		SCARG(&getargs, which) = which;
 		SCARG(&getargs, itv) = oitv;
@@ -591,10 +597,6 @@ sys_setitimer(struct proc *p, void *v, register_t *retval)
 	}
 	if (itvp == 0)
 		return (0);
-	if (itimerfix(&aitv.it_value) || itimerfix(&aitv.it_interval))
-		return (EINVAL);
-	TIMEVAL_TO_TIMESPEC(&aitv.it_value, &aits.it_value);
-	TIMEVAL_TO_TIMESPEC(&aitv.it_interval, &aits.it_interval);
 	if (which == ITIMER_REAL) {
 		struct timespec cts;