On Fri, May 11, 2018 at 05:54:17PM -0500, Scott Cheloha wrote:
> Hi,
>
> This fixes timeout input validation for kevent(2) and nanosleep(2).
>
> nanosleep() and kqueue_scan() (callee of kevent()) both convert the
> incoming timespec to a timeval with TIMESPEC_TO_TIMEVAL *before*
> validating the timeout. This hides invalid tv_nsec values on
> [-999, 0) and (1000000000, 1000000999].
>
> The patch moves the range check up to before the conversion in
> kqueue_scan() and drops the conversion from nanosleep() entirely;
> we use the copied timespec directly instead. It also adds new
> regress tests for both interfaces.
>
> To avoid changes in behavior for both interfaces we need to check
> by hand that tv_sec <= 100 million, as timespecfix() quietly truncates
> tv_sec to 100 million where itimerfix() would have returned an error.
>
> itimerfix() also quietly rounds non-zero tv_usec up to a single tick
> where timespecfix() does not. Thus we need to explicitly round the
> converted timeval in kqueue_scan(). We don't need to round the timespec
> in nanosleep(), though, because tstohz() is wrapped in MAX() which
> ensures we tsleep at least one tick, just like before.
>
> FreeBSD, NetBSD, illumos, and Linux, and all correctly (per the POSIX spec)
> validate the input timespec for nanosleep, so I do not believe this change
> will break anything in ports.
>
> FreeBSD and NetBSD both check the incoming timespec in kqueue_scan()
> against the tighter bound on tv_nsec, so I think it makes sense to do so
> as well.
>
> ok?
I think this is correct and an improvement.
ok tb
(for what that is worth in this part of the tree)
>
> --
> Scott Cheloha
>
> Index: sys/kern/kern_event.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_event.c,v
> retrieving revision 1.88
> diff -u -p -r1.88 kern_event.c
> --- sys/kern/kern_event.c 27 Apr 2018 10:13:37 -0000 1.88
> +++ sys/kern/kern_event.c 11 May 2018 22:49:37 -0000
> @@ -693,6 +693,7 @@ kqueue_scan(struct kqueue *kq, int maxev
> const struct timespec *tsp, struct proc *p, int *retval)
> {
> struct kevent *kevp;
> + struct timespec ats;
> struct timeval atv, rtv, ttv;
> struct knote *kn, marker;
> int s, count, timeout, nkev = 0, error = 0;
> @@ -703,16 +704,18 @@ kqueue_scan(struct kqueue *kq, int maxev
> goto done;
>
> if (tsp != NULL) {
> - TIMESPEC_TO_TIMEVAL(&atv, tsp);
> - if (tsp->tv_sec == 0 && tsp->tv_nsec == 0) {
> + ats = *tsp;
> + if (ats.tv_sec > 100000000 || timespecfix(&ats)) {
> + error = EINVAL;
> + goto done;
> + }
> + TIMESPEC_TO_TIMEVAL(&atv, &ats);
> + if (atv.tv_sec == 0 && atv.tv_usec == 0) {
> /* No timeout, just poll */
> timeout = -1;
> goto start;
> }
> - if (itimerfix(&atv)) {
> - error = EINVAL;
> - goto done;
> - }
> + itimerround(&atv);
>
> timeout = atv.tv_sec > 24 * 60 * 60 ?
> 24 * 60 * 60 * hz : tvtohz(&atv);
> 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 11 May 2018 22:49:37 -0000
> @@ -268,7 +268,6 @@ sys_nanosleep(struct proc *p, void *v, r
> struct timespec rqt, rmt;
> struct timespec sts, ets;
> struct timespec *rmtp;
> - struct timeval tv;
> int error, error1;
>
> rmtp = SCARG(uap, rmtp);
> @@ -283,15 +282,14 @@ sys_nanosleep(struct proc *p, void *v, r
> }
> #endif
>
> - TIMESPEC_TO_TIMEVAL(&tv, &rqt);
> - if (itimerfix(&tv))
> + if (rqt.tv_sec > 100000000 || timespecfix(&rqt))
> return (EINVAL);
>
> if (rmtp)
> getnanouptime(&sts);
>
> error = tsleep(&nanowait, PWAIT | PCATCH, "nanosleep",
> - MAX(1, tvtohz(&tv)));
> + MAX(1, tstohz(&rqt)));
> if (error == ERESTART)
> error = EINTR;
> if (error == EWOULDBLOCK)
> Index: regress/sys/kern/nanosleep/Makefile
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/nanosleep/Makefile,v
> retrieving revision 1.3
> diff -u -p -r1.3 Makefile
> --- regress/sys/kern/nanosleep/Makefile 2 Sep 2002 20:01:44 -0000
> 1.3
> +++ regress/sys/kern/nanosleep/Makefile 11 May 2018 22:49:37 -0000
> @@ -18,7 +18,10 @@ time_elapsed_with_signal: nanosleep
> short_time: nanosleep
> ./nanosleep -S
>
> +invalid_time: nanosleep
> + ./nanosleep -i
> +
> REGRESS_TARGETS=trivial with_signal time_elapsed time_elapsed_with_signal
> -REGRESS_TARGETS+=short_time
> +REGRESS_TARGETS+=short_time invalid_time
>
> .include <bsd.regress.mk>
> Index: regress/sys/kern/nanosleep/nanosleep.c
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/nanosleep/nanosleep.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 nanosleep.c
> --- regress/sys/kern/nanosleep/nanosleep.c 8 Mar 2017 19:28:47 -0000
> 1.6
> +++ regress/sys/kern/nanosleep/nanosleep.c 11 May 2018 22:49:37 -0000
> @@ -6,6 +6,7 @@
> #include <sys/time.h>
> #include <sys/wait.h>
>
> +#include <errno.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> @@ -13,6 +14,7 @@
> #include <err.h>
> #include <signal.h>
>
> +int invalid_time(void);
> int trivial(void);
> int with_signal(void);
> int time_elapsed(void);
> @@ -29,8 +31,11 @@ main(int argc, char **argv)
>
> ret = 0;
>
> - while ((ch = getopt(argc, argv, "tseES")) != -1) {
> + while ((ch = getopt(argc, argv, "itseES")) != -1) {
> switch (ch) {
> + case 'i':
> + ret |= invalid_time();
> + break;
> case 't':
> ret |= trivial();
> break;
> @@ -46,7 +51,7 @@ main(int argc, char **argv)
> case 'S':
> ret |= short_time();
> default:
> - fprintf(stderr, "Usage: nanosleep [-tse]\n");
> + fprintf(stderr, "Usage: nanosleep [-itseSE]\n");
> exit(1);
> }
> }
> @@ -257,5 +262,22 @@ short_time(void)
> if (wait(&status) < 0)
> err(1, "wait");
>
> + return 0;
> +}
> +
> +int
> +invalid_time(void)
> +{
> + struct timespec ts[3] = { {-1, 0}, {0, -1}, {0, 1000000000L} };
> + int i, status;
> +
> + for (i = 0; i < 3; i++) {
> + status = nanosleep(&ts[i], NULL);
> + if (status != -1 || errno != EINVAL) {
> + warnx("invalid-time: nanosleep %lld %ld",
> + (long long)ts[i].tv_sec, ts[i].tv_nsec);
> + return 1;
> + }
> + }
> return 0;
> }
> Index: regress/sys/kern/kqueue/Makefile
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/kqueue/Makefile,v
> retrieving revision 1.19
> diff -u -p -r1.19 Makefile
> --- regress/sys/kern/kqueue/Makefile 20 Sep 2016 23:05:27 -0000 1.19
> +++ regress/sys/kern/kqueue/Makefile 11 May 2018 22:49:37 -0000
> @@ -32,9 +32,11 @@ kq-flock: ${PROG}
> ./${PROG} -l
> kq-timer: ${PROG}
> ./${PROG} -i
> +kq-invalid-timer: ${PROG}
> + ./${PROG} -I
>
> REGRESS_TARGETS=kq-pipe kq-fork kq-process kq-random kq-pty kq-signal \
> - kq-fdpass kq-flock kq-timer
> + kq-fdpass kq-flock kq-timer kq-invalid-timer
> # kq-tun broke at some point, apparently from a change in tun routing
> REGRESS_ROOT_TARGETS=${REGRESS_TARGETS}
> .PHONY: ${REGRESS_TARGETS}
> Index: regress/sys/kern/kqueue/kqueue-timer.c
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/kqueue/kqueue-timer.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 kqueue-timer.c
> --- regress/sys/kern/kqueue/kqueue-timer.c 20 Sep 2016 23:05:27 -0000
> 1.2
> +++ regress/sys/kern/kqueue/kqueue-timer.c 11 May 2018 22:49:37 -0000
> @@ -20,6 +20,7 @@
> #include <sys/event.h>
>
> #include <err.h>
> +#include <errno.h>
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
> @@ -65,6 +66,34 @@ do_timer(void)
>
> n = kevent(kq, NULL, 0, &ev, 1, &ts);
> ASSX(n == 1);
> +
> + return (0);
> +}
> +
> +int
> +do_invalid_timer(void)
> +{
> + int i, kq, n;
> + struct kevent ev;
> + struct timespec invalid_ts[3] = { {-1, 0}, {0, -1}, {0, 1000000000L} };
> +
> + ASS((kq = kqueue()) >= 0,
> + warn("kqueue"));
> +
> + memset(&ev, 0, sizeof(ev));
> + ev.filter = EVFILT_TIMER;
> + ev.flags = EV_ADD | EV_ENABLE;
> + ev.data = 500; /* 1/2 second in ms */
> +
> + n = kevent(kq, &ev, 1, NULL, 0, NULL);
> + ASSX(n != -1);
> +
> + for (i = 0; i < 3; i++) {
> + n = kevent(kq, NULL, 0, &ev, 1, &invalid_ts[i]);
> + ASS(n == -1 && errno == EINVAL,
> + warn("kevent: timeout %lld %ld",
> + (long long)invalid_ts[i].tv_sec, invalid_ts[i].tv_nsec));
> + }
>
> return (0);
> }
> Index: regress/sys/kern/kqueue/main.c
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/kqueue/main.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 main.c
> --- regress/sys/kern/kqueue/main.c 20 Sep 2016 23:05:27 -0000 1.9
> +++ regress/sys/kern/kqueue/main.c 11 May 2018 22:49:37 -0000
> @@ -16,7 +16,7 @@ main(int argc, char **argv)
> int ret, c;
>
> ret = 0;
> - while ((c = getopt(argc, argv, "fFilpPrstT")) != -1) {
> + while ((c = getopt(argc, argv, "fFiIlpPrstT")) != -1) {
> switch (c) {
> case 'f':
> ret |= check_inheritance();
> @@ -26,6 +26,9 @@ main(int argc, char **argv)
> break;
> case 'i':
> ret |= do_timer();
> + break;
> + case 'I':
> + ret |= do_invalid_timer();
> break;
> case 'l':
> ret |= do_flock();
> Index: regress/sys/kern/kqueue/main.h
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/kqueue/main.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 main.h
> --- regress/sys/kern/kqueue/main.h 20 Sep 2016 23:05:27 -0000 1.1
> +++ regress/sys/kern/kqueue/main.h 11 May 2018 22:49:37 -0000
> @@ -18,6 +18,7 @@
> int check_inheritance(void);
> int do_fdpass(void);
> int do_flock(void);
> +int do_invalid_timer(void);
> int do_pipe(void);
> int do_process(void);
> int do_pty(void);
>
