[PATCH] 'select' failure or signal should not update timeout

[Paul Eggert <eggert@twinsun.com>]

[This follows up a thread in the emacs-pretesters mailing list about a
problem with Emacs, 'select', SA_RESTART, and interrupts.]

> Date: Wed, 17 Jul 2002 08:30:57 -0600 (MDT)
> From: Richard Stallman <rms@gnu.org>
> 
> Is it true today that restarting a `select' call after a signal (with
> SA_RESTART) alters the contents of the user's timeout parameter?

I looked into this a bit more, and it turns out to be a problem in the
Linux kernel.  POSIX 1003.1-2001
<http://www.opengroup.org/onlinepubs/007904975/functions/select.html>
says that 'select' may modify its timeout argument only "upon
successful completion".  However, the Linux kernel sometimes modifies
the timeout argument even when 'select' fails or is interrupted.

Here is a program that illustrates the problem.  At the end of this
message I enclose a proposed patch to the Linux kernel to fix the
problem.

	/* Conformance test for POSIX 1003.1-2001's requirement that select()
	   must not update its timeout argument unless it succeeds.  */

	#include <signal.h>
	#include <stdio.h>
	#include <stdlib.h>
	#include <sys/time.h>
	#include <unistd.h>

	static void
	moan (char const *string)
	{
	  perror (string);
	  exit (1);
	}

	struct timeval timeout;
	struct timeval timeout_when_in_handler;
	struct sigaction act;

	void
	handle_sigalrm (int sig)
	{
	  timeout_when_in_handler = timeout;
	}

	enum { TIMEOUT_SECONDS = 5 };

	int
	main (int argc, char **argv)
	{
	  act.sa_handler = handle_sigalrm;
	  act.sa_flags = SA_RESTART;
	  if (sigaction (SIGALRM, &act, 0) != 0)
	    moan ("sigaction");
	  timeout.tv_sec = TIMEOUT_SECONDS;
	  timeout_when_in_handler = timeout;
	  alarm (TIMEOUT_SECONDS / 2);
	  if (select (0, 0, 0, 0, &timeout) != 0)
	    {
	      if (timeout.tv_sec != TIMEOUT_SECONDS)
		{
		  perror ("select");
		  fprintf (stderr, "select failed, but timeout was updated to %ld.%.9ld seconds\n",
			   (long) timeout.tv_sec, timeout.tv_usec);
		}
	    }

	  if (timeout_when_in_handler.tv_sec != TIMEOUT_SECONDS)
	    fprintf (stderr, "timeout was updated to %ld.%.9ld seconds while signal handler was active\n",
		     (long) timeout_when_in_handler.tv_sec,
		     timeout_when_in_handler.tv_usec);

	  return 0;
	}


Here is a proposed patch to Linux kernel 2.5.26.  The patch also
applies to Linux 2.4.18, though you have to ignore the patches to
files that do not exist in 2.4.18.  I haven't tested this patch, but
it's pretty straightforward.

diff -prU6 2.5.26/arch/ia64/ia32/sys_ia32.c 2.5.26-select/arch/ia64/ia32/sys_ia32.c
--- 2.5.26/arch/ia64/ia32/sys_ia32.c	Tue Jul 16 16:49:35 2002
+++ 2.5.26-select/arch/ia64/ia32/sys_ia32.c	Fri Jul 19 02:17:08 2002
@@ -1058,32 +1058,32 @@ sys32_select (int n, fd_set *inp, fd_set
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp32 && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		if (put_user(sec, &tvp32->tv_sec) || put_user(usec, &tvp32->tv_usec)) {
 			ret = -EFAULT;
 			goto out;
 		}
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set(n, inp, fds.res_in);
 	set_fd_set(n, outp, fds.res_out);
 	set_fd_set(n, exp, fds.res_ex);
 
diff -prU6 2.5.26/arch/mips64/kernel/linux32.c 2.5.26-select/arch/mips64/kernel/linux32.c
--- 2.5.26/arch/mips64/kernel/linux32.c	Tue Jul 16 16:49:25 2002
+++ 2.5.26-select/arch/mips64/kernel/linux32.c	Fri Jul 19 02:18:19 2002
@@ -1170,30 +1170,30 @@ asmlinkage int sys32_select(int n, u32 *
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, &tvp->tv_sec);
 		put_user(usec, &tvp->tv_usec);
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set32(nn, inp, fds.res_in);
 	set_fd_set32(nn, outp, fds.res_out);
 	set_fd_set32(nn, exp, fds.res_ex);
 
diff -prU6 2.5.26/arch/ppc64/kernel/sys_ppc32.c 2.5.26-select/arch/ppc64/kernel/sys_ppc32.c
--- 2.5.26/arch/ppc64/kernel/sys_ppc32.c	Tue Jul 16 16:49:36 2002
+++ 2.5.26-select/arch/ppc64/kernel/sys_ppc32.c	Fri Jul 19 02:17:58 2002
@@ -762,30 +762,30 @@ asmlinkage long sys32_select(int n, u32 
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+  if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, &tvp->tv_sec);
 		put_user(usec, &tvp->tv_usec);
-	}
-
-  if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set32(nn, inp, fds.res_in);
 	set_fd_set32(nn, outp, fds.res_out);
 	set_fd_set32(nn, exp, fds.res_ex);
   
diff -prU6 2.5.26/arch/s390x/kernel/linux32.c 2.5.26-select/arch/s390x/kernel/linux32.c
--- 2.5.26/arch/s390x/kernel/linux32.c	Tue Jul 16 16:49:27 2002
+++ 2.5.26-select/arch/s390x/kernel/linux32.c	Fri Jul 19 02:18:27 2002
@@ -1420,30 +1420,30 @@ asmlinkage int sys32_select(int n, u32 *
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp && !(current->personality & STICKY_TIMEOUTS)) {
 		int sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, &tvp->tv_sec);
 		put_user(usec, &tvp->tv_usec);
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set32(nn, inp, fds.res_in);
 	set_fd_set32(nn, outp, fds.res_out);
 	set_fd_set32(nn, exp, fds.res_ex);
 
diff -prU6 2.5.26/arch/sparc64/kernel/sys_sparc32.c 2.5.26-select/arch/sparc64/kernel/sys_sparc32.c
--- 2.5.26/arch/sparc64/kernel/sys_sparc32.c	Tue Jul 16 16:49:35 2002
+++ 2.5.26-select/arch/sparc64/kernel/sys_sparc32.c	Fri Jul 19 02:16:59 2002
@@ -1387,30 +1387,30 @@ asmlinkage int sys32_select(int n, u32 *
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, &tvp->tv_sec);
 		put_user(usec, &tvp->tv_usec);
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set32(nn, inp, fds.res_in);
 	set_fd_set32(nn, outp, fds.res_out);
 	set_fd_set32(nn, exp, fds.res_ex);
 
diff -prU6 2.5.26/arch/x86_64/ia32/sys_ia32.c 2.5.26-select/arch/x86_64/ia32/sys_ia32.c
--- 2.5.26/arch/x86_64/ia32/sys_ia32.c	Tue Jul 16 16:49:32 2002
+++ 2.5.26-select/arch/x86_64/ia32/sys_ia32.c	Fri Jul 19 02:18:07 2002
@@ -878,30 +878,30 @@ sys32_select(int n, fd_set *inp, fd_set 
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp32 && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, (int *)&tvp32->tv_sec);
 		put_user(usec, (int *)&tvp32->tv_usec);
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set(n, inp, fds.res_in);
 	set_fd_set(n, outp, fds.res_out);
 	set_fd_set(n, exp, fds.res_ex);
 
diff -prU6 2.5.26/fs/select.c 2.5.26-select/fs/select.c
--- 2.5.26/fs/select.c	Tue Jul 16 16:49:24 2002
+++ 2.5.26-select/fs/select.c	Fri Jul 19 02:18:41 2002
@@ -316,30 +316,30 @@ sys_select(int n, fd_set *inp, fd_set *o
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
 
 	ret = do_select(n, &fds, &timeout);
 
+	if (ret < 0)
+		goto out;
+	if (!ret) {
+		ret = -ERESTARTNOHAND;
+		if (signal_pending(current))
+			goto out;
+		ret = 0;
+	}
+
 	if (tvp && !(current->personality & STICKY_TIMEOUTS)) {
 		time_t sec = 0, usec = 0;
 		if (timeout) {
 			sec = timeout / HZ;
 			usec = timeout % HZ;
 			usec *= (1000000/HZ);
 		}
 		put_user(sec, &tvp->tv_sec);
 		put_user(usec, &tvp->tv_usec);
-	}
-
-	if (ret < 0)
-		goto out;
-	if (!ret) {
-		ret = -ERESTARTNOHAND;
-		if (signal_pending(current))
-			goto out;
-		ret = 0;
 	}
 
 	set_fd_set(n, inp, fds.res_in);
 	set_fd_set(n, outp, fds.res_out);
 	set_fd_set(n, exp, fds.res_ex);