Current pselect(3) is implemented in usermode and thus vulnerable to

well-known race condition, which elimination was the reason for the
function appearance in first place. If sigmask supplied as argument to
pselect() enables a signal, the signal might be delivered before thread
called select(2), causing lost wakeup. Reimplement pselect() in kernel,
making change of sigmask and sleep atomic.

Since signal shall be delivered to the usermode, but sigmask restored,
set TDP_OLDMASK and save old mask in td_oldsigmask. The TDP_OLDMASK
should be cleared by ast() in case signal was not gelivered during
syscall execution.

Reviewed by:	davidxu
Tested by:	pho
MFC after:	1 month

9 files changed, 109 insertions, 82 deletions

diff --git a/lib/libc/gen/pselect.c b/lib/libc/gen/pselect.c
deleted file mode 100644
index 28066a2..0000000
--- a/lib/libc/gen/pselect.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * Copyright 2000 Massachusetts Institute of Technology
- *
- * Permission to use, copy, modify, and distribute this software and
- * its documentation for any purpose and without fee is hereby
- * granted, provided that both the above copyright notice and this
- * permission notice appear in all copies, that both the above
- * copyright notice and this permission notice appear in all
- * supporting documentation, and that the name of M.I.T. not be used
- * in advertising or publicity pertaining to distribution of the
- * software without specific, written prior permission.  M.I.T. makes
- * no representations about the suitability of this software for any
- * purpose.  It is provided "as is" without express or implied
- * warranty.
- *
- * THIS SOFTWARE IS PROVIDED BY M.I.T. ``AS IS''.  M.I.T. DISCLAIMS
- * ALL EXPRESS OR IMPLIED WARRANTIES WITH REGARD TO THIS SOFTWARE,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. IN NO EVENT
- * SHALL M.I.T. BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
- * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
- * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/cdefs.h>
-__FBSDID("$FreeBSD$");
-
-#include "namespace.h"
-#include <sys/select.h>
-#include <sys/time.h>
-
-#include <errno.h>
-#include <signal.h>
-#include "un-namespace.h"
-
-__weak_reference(__pselect, pselect);
-
-/*
- * Emulate the POSIX 1003.1g-2000 `pselect' interface.  This is the
- * same as the traditional BSD `select' function, except that it uses
- * a timespec rather than a timeval, doesn't modify the timeout argument,
- * and allows the user to specify a signal mask to apply during the select.
- */
-int
-__pselect(int count, fd_set * __restrict rfds, fd_set * __restrict wfds, 
-	fd_set * __restrict efds, const struct timespec * __restrict timo, 
-	const sigset_t * __restrict mask)
-{
-	sigset_t omask;
-	struct timeval tvtimo, *tvp;
-	int rv, sverrno;
-
-	if (timo) {
-		TIMESPEC_TO_TIMEVAL(&tvtimo, timo);
-		tvp = &tvtimo;
-	} else
-		tvp = 0;
-
-	if (mask != 0) {
-		rv = _sigprocmask(SIG_SETMASK, mask, &omask);
-		if (rv != 0)
-			return rv;
-	}
-
-	rv = _select(count, rfds, wfds, efds, tvp);
-	if (mask != 0) {
-		sverrno = errno;
-		_sigprocmask(SIG_SETMASK, &omask, (sigset_t *)0);
-		errno = sverrno;
-	}
-
-	return rv;
-}
diff --git a/lib/libc/sys/Symbol.map b/lib/libc/sys/Symbol.map
index c834a25..ce6f32a 100644
--- a/lib/libc/sys/Symbol.map
+++ b/lib/libc/sys/Symbol.map
@@ -211,6 +211,7 @@ FBSD_1.0 {
 	posix_openpt;
 	preadv;
 	profil;
+	pselect;
 	ptrace;
 	pwritev;
 	quotactl;
@@ -781,6 +782,8 @@ FBSDprivate_1.0 {
 	__sys_preadv;
 	_profil;
 	__sys_profil;
+	_pselect;
+	__sys_pselect;
 	_ptrace;
 	__sys_ptrace;
 	_pwritev;
diff --git a/lib/libthr/thread/thr_syscalls.c b/lib/libthr/thread/thr_syscalls.c
index d05d68b..92670a9 100644
--- a/lib/libthr/thread/thr_syscalls.c
+++ b/lib/libthr/thread/thr_syscalls.c
@@ -104,6 +104,8 @@ extern int	__sys_accept(int, struct sockaddr *, socklen_t *);
 extern int	__sys_connect(int, const struct sockaddr *, socklen_t);
 extern int	__sys_fsync(int);
 extern int	__sys_msync(void *, size_t, int);
+extern int	__sys_pselect(int, fd_set *, fd_set *, fd_set *,
+			const struct timespec *, const sigset_t *);
 extern int	__sys_poll(struct pollfd *, unsigned, int);
 extern ssize_t	__sys_recv(int, void *, size_t, int);
 extern ssize_t	__sys_recvfrom(int, void *, size_t, int, struct sockaddr *, socklen_t *);
@@ -394,7 +396,7 @@ ___pselect(int count, fd_set *rfds, fd_set *wfds, fd_set *efds,
 	int ret;
 
 	_thr_cancel_enter(curthread);
-	ret = __pselect(count, rfds, wfds, efds, timo, mask);
+	ret = __sys_pselect(count, rfds, wfds, efds, timo, mask);
 	_thr_cancel_leave(curthread);
 
 	return (ret);
diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c
index 37fa079..75b290b 100644
--- a/sys/compat/freebsd32/freebsd32_misc.c
+++ b/sys/compat/freebsd32/freebsd32_misc.c
@@ -593,6 +593,41 @@ freebsd32_select(struct thread *td, struct freebsd32_select_args *uap)
 	    sizeof(int32_t) * 8));
 }
 
+int
+freebsd32_pselect(struct thread *td, struct freebsd32_pselect_args *uap)
+{
+	struct timespec32 ts32;
+	struct timespec ts;
+	struct timeval tv, *tvp;
+	sigset_t set, *uset;
+	int error;
+
+	if (uap->ts != NULL) {
+		error = copyin(uap->ts, &ts32, sizeof(ts32));
+		if (error != 0)
+			return (error);
+		CP(ts32, ts, tv_sec);
+		CP(ts32, ts, tv_nsec);
+		TIMESPEC_TO_TIMEVAL(&tv, &ts);
+		tvp = &tv;
+	} else
+		tvp = NULL;
+	if (uap->sm != NULL) {
+		error = copyin(uap->sm, &set, sizeof(set));
+		if (error != 0)
+			return (error);
+		uset = &set;
+	} else
+		uset = NULL;
+	/*
+	 * XXX big-endian needs to convert the fd_sets too.
+	 * XXX Do pointers need PTRIN()?
+	 */
+	error = kern_pselect(td, uap->nd, uap->in, uap->ou, uap->ex, tvp,
+	    uset, sizeof(int32_t) * 8);
+	return (error);
+}
+
 /*
  * Copy 'count' items into the destination list pointed to by uap->eventlist.
  */
diff --git a/sys/compat/freebsd32/syscalls.master b/sys/compat/freebsd32/syscalls.master
index 833b1e5..865f0c1 100644
--- a/sys/compat/freebsd32/syscalls.master
+++ b/sys/compat/freebsd32/syscalls.master
@@ -909,3 +909,7 @@
 519	AUE_PDKILL	UNIMPL	pdkill
 520	AUE_PDGETPID	UNIMPL	pdgetpid
 521	AUE_PDWAIT	UNIMPL	pdwait
+522	AUE_SELECT	STD	{ int freebsd32_pselect(int nd, fd_set *in, \
+				    fd_set *ou, fd_set *ex, \
+				    const struct timespec32 *ts, \
+				    const sigset_t *sm); }
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index ccf6479..4d20ebd 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -245,6 +245,11 @@ ast(struct trapframe *framep)
 		PROC_UNLOCK(p);
 	}
 
+	if (td->td_pflags & TDP_OLDMASK) {
+		td->td_pflags &= ~TDP_OLDMASK;
+		kern_sigprocmask(td, SIG_SETMASK, &td->td_oldsigmask, NULL, 0);
+	}
+
 	userret(td, framep);
 	mtx_assert(&Giant, MA_NOTOWNED);
 }
diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c
index 6831fe8..b34af61 100644
--- a/sys/kern/sys_generic.c
+++ b/sys/kern/sys_generic.c
@@ -751,6 +751,58 @@ poll_no_poll(int events)
 	return (events & (POLLIN | POLLOUT | POLLRDNORM | POLLWRNORM));
 }
 
+int
+pselect(struct thread *td, struct pselect_args *uap)
+{
+	struct timespec ts;
+	struct timeval tv, *tvp;
+	sigset_t set, *uset;
+	int error;
+
+	if (uap->ts != NULL) {
+		error = copyin(uap->ts, &ts, sizeof(ts));
+		if (error != 0)
+		    return (error);
+		TIMESPEC_TO_TIMEVAL(&tv, &ts);
+		tvp = &tv;
+	} else
+		tvp = NULL;
+	if (uap->sm != NULL) {
+		error = copyin(uap->sm, &set, sizeof(set));
+		if (error != 0)
+			return (error);
+		uset = &set;
+	} else
+		uset = NULL;
+	return (kern_pselect(td, uap->nd, uap->in, uap->ou, uap->ex, tvp,
+	    uset, NFDBITS));
+}
+
+int
+kern_pselect(struct thread *td, int nd, fd_set *in, fd_set *ou, fd_set *ex,
+    struct timeval *tvp, sigset_t *uset, int abi_nfdbits)
+{
+	int error;
+
+	if (uset != NULL) {
+		error = kern_sigprocmask(td, SIG_SETMASK, uset,
+		    &td->td_oldsigmask, 0);
+		if (error != 0)
+			return (error);
+		td->td_pflags |= TDP_OLDMASK;
+		/*
+		 * Make sure that ast() is called on return to
+		 * usermode and TDP_OLDMASK is cleared, restoring old
+		 * sigmask.
+		 */
+		thread_lock(td);
+		td->td_flags |= TDF_ASTPENDING;
+		thread_unlock(td);
+	}
+	error = kern_select(td, nd, in, ou, ex, tvp, abi_nfdbits);
+	return (error);
+}
+
 #ifndef _SYS_SYSPROTO_H_
 struct select_args {
 	int	nd;
@@ -759,9 +811,7 @@ struct select_args {
 };
 #endif
 int
-select(td, uap)
-	register struct thread *td;
-	register struct select_args *uap;
+select(struct thread *td, struct select_args *uap)
 {
 	struct timeval tv, *tvp;
 	int error;
diff --git a/sys/kern/syscalls.master b/sys/kern/syscalls.master
index f96ed79..ac64a75 100644
--- a/sys/kern/syscalls.master
+++ b/sys/kern/syscalls.master
@@ -919,5 +919,9 @@
 519	AUE_PDKILL	UNIMPL	pdkill
 520	AUE_PDGETPID	UNIMPL	pdgetpid
 521	AUE_PDWAIT	UNIMPL	pdwait
+522	AUE_SELECT	STD	{ int pselect(int nd, fd_set *in, \
+				    fd_set *ou, fd_set *ex, \
+				    const struct timespec *ts, \
+				    const sigset_t *sm); }
 ; Please copy any additions and changes to the following compatability tables:
 ; sys/compat/freebsd32/syscalls.master
diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h
index c389873..2813a58 100644
--- a/sys/sys/syscallsubr.h
+++ b/sys/sys/syscallsubr.h
@@ -148,6 +148,8 @@ int	kern_pathconf(struct thread *td, char *path, enum uio_seg pathseg,
 	    int name, u_long flags);
 int	kern_pipe(struct thread *td, int fildes[2]);
 int	kern_preadv(struct thread *td, int fd, struct uio *auio, off_t offset);
+int	kern_pselect(struct thread *td, int nd, fd_set *in, fd_set *ou,
+	    fd_set *ex, struct timeval *tvp, sigset_t *uset, int abi_nfdbits);
 int	kern_ptrace(struct thread *td, int req, pid_t pid, void *addr,
 	    int data);
 int	kern_pwritev(struct thread *td, int fd, struct uio *auio, off_t offset);