On Thu, May 10, 2018 at 09:13:27AM -0500, Scott Cheloha wrote:
> Hi,
>
> This sets EINVAL on non-normal input for adjtime(2), clock_settime(2),
> and settimeofday(2).
>
> clock_settime and settimeofday take absolute times as input, and adjtime
> is explicitly allowed to be a negative relative time, so timespecfix() is
> inapplicable.
>
> POSIX specifies that clock_settime(2) set EINVAL for non-normal input,
> and we document this but don't actually do the check. FreeBSD, NetBSD,
> illumos, and Linux all check and set EINVAL.
>
> Although adjtime(2) and settimeofday(2) are non-standard, the timeval
> struct itself *is* specified by POSIX, and (almost?) every standard
> system call using a timeval or a timespec is specified to set EINVAL
> for non-normal input. FreeBSD, illumos, and Linux set EINVAL for
> adjtime(2) and settimeofday(2).
>
> Maybe of note is that illumos sets EINVAL for adjtime(2) but that their
> valid range for tv_usec is (-1000000, 1000000), which seems off: I'm
> pretty sure you can represent all relevant absolute subseconds while
> constraining tv_usec to [0, 1000000), the valid range used effectively
> everywhere else.
>
> If anything, consistent behavior is good, and most system calls validate
> timeval/timespec inputs, so I think we ought to do the same for these.
>
> ok?
>
> --
> Scott Cheloha
>
> jmc: Is past tense preferred in ERRORS?
>
> i.e. "<arg> specified" vs. "<arg> specifies"
>
there's no policy i'm aware of. i think having the page consistent is
more important. i guess past tense often sounds better for these (to my
ears anyway), but i don;t think it matters much.
jmc
> Index: lib/libc/sys/adjtime.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/adjtime.2,v
> retrieving revision 1.22
> diff -u -p -r1.22 adjtime.2
> --- lib/libc/sys/adjtime.2 10 Sep 2015 17:55:21 -0000 1.22
> +++ lib/libc/sys/adjtime.2 10 May 2018 14:12:42 -0000
> @@ -87,6 +87,11 @@ will fail if:
> .Bl -tag -width Er
> .It Bq Er EFAULT
> Either of the arguments point outside the process's allocated address space.
> +.It Bq Er EINVAL
> +The
> +.Fn delta
> +argument specifies a microsecond value less than zero or
> +greater than or equal to 1 million.
> .It Bq Er EPERM
> The
> .Fn delta
> Index: lib/libc/sys/clock_gettime.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/clock_gettime.2,v
> retrieving revision 1.29
> diff -u -p -r1.29 clock_gettime.2
> --- lib/libc/sys/clock_gettime.2 18 Dec 2017 07:15:15 -0000 1.29
> +++ lib/libc/sys/clock_gettime.2 10 May 2018 14:12:42 -0000
> @@ -139,7 +139,8 @@ A user other than the superuser attempte
> .Fa clock_id
> specifies a clock that isn't settable,
> .Fa tp
> -specifies a nanosecond value less than zero or greater than 1000 million,
> +specifies a nanosecond value less than zero or
> +greater than or equal to 1000 million,
> or a value outside the range of the specified clock.
> .El
> .Sh SEE ALSO
> Index: lib/libc/sys/gettimeofday.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/gettimeofday.2,v
> retrieving revision 1.29
> diff -u -p -r1.29 gettimeofday.2
> --- lib/libc/sys/gettimeofday.2 10 Sep 2015 17:55:21 -0000 1.29
> +++ lib/libc/sys/gettimeofday.2 10 May 2018 14:12:42 -0000
> @@ -118,8 +118,12 @@ An argument address referenced invalid m
> .Pp
> In addition,
> .Fn settimeofday
> -may return the following error:
> +may return the following errors:
> .Bl -tag -width Er
> +.It Bq Er EINVAL
> +.Fa tp
> +specified a microsecond value less than zero or
> +greater than or equal to 1 million.
> .It Bq Er EPERM
> A user other than the superuser attempted to set the time.
> .El
> Index: sys/kern/kern_time.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 kern_time.c
> --- sys/kern/kern_time.c 19 Feb 2018 08:59:52 -0000 1.101
> +++ sys/kern/kern_time.c 10 May 2018 14:12:42 -0000
> @@ -197,6 +197,8 @@ sys_clock_settime(struct proc *p, void *
> clock_id = SCARG(uap, clock_id);
> switch (clock_id) {
> case CLOCK_REALTIME:
> + if (ats.tv_nsec < 0 || ats.tv_nsec >= 1000000000)
> + return (EINVAL);
> if ((error = settime(&ats)) != 0)
> return (error);
> break;
> @@ -381,6 +383,8 @@ sys_settimeofday(struct proc *p, void *v
> if (tv) {
> struct timespec ts;
>
> + if (atv.tv_usec < 0 || atv.tv_usec >= 1000000)
> + return (EINVAL);
> TIMEVAL_TO_TIMESPEC(&atv, &ts);
> if ((error = settime(&ts)) != 0)
> return (error);
> @@ -453,6 +457,8 @@ sys_adjtime(struct proc *p, void *v, reg
>
> if ((error = copyin(delta, &atv, sizeof(struct timeval))))
> return (error);
> + if (atv.tv_usec < 0 || atv.tv_usec >= 1000000)
> + return (EINVAL);
>
> /* XXX Check for overflow? */
> adjtimedelta = (int64_t)atv.tv_sec * 1000000 + atv.tv_usec;
>
