I don't agree. While the 2nd parameter is basically a dud, there is still an order of erroring reporting here, and you are changing that order.
Right now it tests for the EFAULT conditions on both parameters. You change the order into EFAULT, EINVAL, EFAULT, etc. Right now, it clusters the copyin's of all the parameters to above, so that a non-copyin sleeping-type operation can't intervene. I don't see reason to change that. And you are moving the ktrace code, it seems in error cases this will now behave different (it can report a ktrace, where before it would EFAULT early). I don't see the justification for changing the order of code from what it was in -r1.1 Scott Cheloha <[email protected]> wrote: > Unless there's a good reason to wait, we ought to validate the input > as soon as we have it. There is no reason to wait to validate the > input time in this context. > > ok? > > Index: kern_time.c > =================================================================== > RCS file: /cvs/src/sys/kern/kern_time.c,v > retrieving revision 1.159 > diff -u -p -r1.159 kern_time.c > --- kern_time.c 5 Dec 2022 23:18:37 -0000 1.159 > +++ kern_time.c 14 Dec 2022 23:51:14 -0000 > @@ -375,19 +375,22 @@ sys_settimeofday(struct proc *p, void *v > if ((error = suser(p))) > return (error); > /* Verify all parameters before changing time. */ > - if (tv && (error = copyin(tv, &atv, sizeof(atv)))) > - return (error); > - if (tzp && (error = copyin(tzp, &atz, sizeof(atz)))) > - return (error); > - if (tv) { > - struct timespec ts; > - > + if (tv != NULL) { > + error = copyin(tv, &atv, sizeof(atv)); > + if (error) > + return error; > #ifdef KTRACE > if (KTRPOINT(p, KTR_STRUCT)) > ktrabstimeval(p, &atv); > #endif > if (!timerisvalid(&atv)) > return (EINVAL); > + } > + if (tzp && (error = copyin(tzp, &atz, sizeof(atz)))) > + return (error); > + if (tv) { > + struct timespec ts; > + > TIMEVAL_TO_TIMESPEC(&atv, &ts); > if ((error = settime(&ts)) != 0) > return (error); >
