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

From: Paul Eggert (eggert@twinsun.com)
Date: Fri Jul 19 2002 - 04:52:04 EST


[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);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Jul 23 2002 - 22:00:29 EST