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?
--
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);
