[PATCH] select: reject invalid file descriptors

[Eric Blake]

POSIX requires invalid file descriptors to be detected rather than
silently ignored.  FreeBSD 8.2 detects if fd 0 has been closed
and appears in a set while fd 1 is still open, but mistakenly
optimizes and refuses to check any fds in the set beyond the
maximum open fd.

* doc/posix-functions/select.texi (select): Document this.
* m4/select.m4 (gl_FUNC_SELECT): Probe for FreeBSD bug.
* lib/select.c (rpl_select) [!win32]: Work around it.
* modules/select (Depends-on): Add dup2.
* tests/test-select.h (do_select_bad_nfd_nowait, test_bad_nfd):
New functions.
(test_function): Enhance test.
(do_select_bad_fd): Avoid any stale errno values.
---

This plugs the remaining 'make check' failure that I saw for
FreeBSD 8.2 when building libvirt.  pselect also has the bug,
so I'm still working on that patch.

 ChangeLog                       | 10 ++++++++++
 doc/posix-functions/select.texi |  4 ++++
 lib/select.c                    | 19 +++++++++++++++++++
 m4/select.m4                    | 40 +++++++++++++++++++++++++++++++++++++++-
 modules/select                  |  1 +
 tests/test-select.h             | 25 +++++++++++++++++++++++++
 6 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index c27db0d..36511c3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2012-10-02  Eric Blake  <address@hidden>

+       select: reject invalid file descriptors
+       * doc/posix-functions/select.texi (select): Document this.
+       * m4/select.m4 (gl_FUNC_SELECT): Probe for FreeBSD bug.
+       * lib/select.c (rpl_select) [!win32]: Work around it.
+       * modules/select (Depends-on): Add dup2.
+       * tests/test-select.h (do_select_bad_nfd_nowait, test_bad_nfd):
+       New functions.
+       (test_function): Enhance test.
+       (do_select_bad_fd): Avoid any stale errno values.
+
        ptsname: reject invalid file descriptors
        http://www.austingroupbugs.net/view.php?id=503
        * m4/ptsname.m4 (gl_FUNC_PTSNAME): Probe for FreeBSD bug.
diff --git a/doc/posix-functions/select.texi b/doc/posix-functions/select.texi
index c13b30f..26fb202 100644
--- a/doc/posix-functions/select.texi
+++ b/doc/posix-functions/select.texi
@@ -18,6 +18,10 @@ select
 @item
 This function fails when the @code{nfds} argument is 0 on some platforms:
 Interix 3.5.
address@hidden
+On some platforms, this function fails to detect invalid fds with
+EBADF, but only if they lie beyond the current maximum open fd:
+FreeBSD 8.2.
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/select.c b/lib/select.c
index 4db09a3..07cc886 100644
--- a/lib/select.c
+++ b/lib/select.c
@@ -507,6 +507,8 @@ restart:

 #include <sys/select.h>
 #include <stddef.h> /* NULL */
+#include <errno.h>
+#include <unistd.h>

 #undef select

@@ -514,6 +516,23 @@ int
 rpl_select (int nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds,
             struct timeval *timeout)
 {
+  int i;
+
+  /* FreeBSD 8.2 has a bug: it does not detect invalid fds.  */
+  if (nfds < 0 || nfds > FD_SETSIZE)
+    {
+      errno = EINVAL;
+      return -1;
+    }
+  for (i = 0; i < nfds; i++)
+    {
+      if (((rfds && FD_ISSET (i, rfds))
+           || (wfds && FD_ISSET (i, wfds))
+           || (xfds && FD_ISSET (i, xfds)))
+          && dup2 (i, i) != i)
+        return -1;
+    }
+
   /* Interix 3.5 has a bug: it does not support nfds == 0.  */
   if (nfds == 0)
     {
diff --git a/m4/select.m4 b/m4/select.m4
index 037b3d3..493bce1 100644
--- a/m4/select.m4
+++ b/m4/select.m4
@@ -1,4 +1,4 @@
-# select.m4 serial 6
+# select.m4 serial 7
 dnl Copyright (C) 2009-2012 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -46,6 +46,44 @@ changequote([,])dnl
       *yes) ;;
       *) REPLACE_SELECT=1 ;;
     esac
+
+    dnl On FreeBSD 8.2, select() doesn't reject bad fds.
+    AC_CACHE_CHECK([whether select detects invalid fds],
+      [gl_cv_func_select_detects_ebadf],
+      [
+        AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#include <sys/types.h>
+#include <sys/time.h>
+#if HAVE_SYS_SELECT_H
+# include <sys/select.h>
+#endif
+#include <unistd.h>
+#include <errno.h>
+]],[[
+  fd_set set;
+  dup2(0, 16);
+  FD_ZERO(&set);
+  FD_SET(16, &set);
+  close(16);
+  struct timeval timeout;
+  timeout.tv_sec = 0;
+  timeout.tv_usec = 5;
+  return select (17, &set, NULL, NULL, &timeout) != -1 || errno != EBADF;
+]])], [gl_cv_func_select_detects_ebadf=yes],
+      [gl_cv_func_select_detects_ebadf=no],
+          [
+           case "$host_os" in
+                    # Guess yes on glibc systems.
+            *-gnu*) gl_cv_func_select_detects_ebadf="guessing yes" ;;
+                    # If we don't know, assume the worst.
+            *)      gl_cv_func_select_detects_ebadf="guessing no" ;;
+           esac
+          ])
+      ])
+    case $gl_cv_func_select_detects_ebadf in
+      *yes) ;;
+      *) REPLACE_SELECT=1 ;;
+    esac
   fi

   dnl Determine the needed libraries.
diff --git a/modules/select b/modules/select
index ab8ce7e..28c89ee 100644
--- a/modules/select
+++ b/modules/select
@@ -8,6 +8,7 @@ m4/select.m4
 Depends-on:
 sys_select
 alloca          [test $REPLACE_SELECT = 1]
+dup2            [test $REPLACE_SELECT = 1]
 sockets         [test $REPLACE_SELECT = 1]
 sys_time        [test $REPLACE_SELECT = 1]
 msvc-nothrow    [test $REPLACE_SELECT = 1]
diff --git a/tests/test-select.h b/tests/test-select.h
index af0e38c..e9cb7d0 100644
--- a/tests/test-select.h
+++ b/tests/test-select.h
@@ -227,6 +227,29 @@ test_tty (select_fn my_select)
 #endif


+static int
+do_select_bad_nfd_nowait (int nfd, select_fn my_select)
+{
+  struct timeval tv0;
+  tv0.tv_sec = 0;
+  tv0.tv_usec = 0;
+  errno = 0;
+  return my_select (nfd, NULL, NULL, NULL, &tv0);
+}
+
+static void
+test_bad_nfd (select_fn my_select)
+{
+  if (do_select_bad_nfd_nowait (-1, my_select) != -1 || errno != EINVAL)
+    failed ("invalid errno after negative nfds");
+  /* Can't test FD_SETSIZE + 1 for EINVAL, since some systems allow
+     dynamically larger set size by redefining FD_SETSIZE anywhere up
+     to the actual maximum fd.  */
+  /* if (do_select_bad_nfd_nowait (FD_SETSIZE + 1, my_select) != -1 */
+  /*     || errno != EINVAL) */
+  /*   failed ("invalid errno after bogus nfds"); */
+}
+
 /* Test select(2) on invalid file descriptors.  */

 static int
@@ -243,6 +266,7 @@ do_select_bad_fd (int fd, int ev, struct timeval *timeout, 
select_fn my_select)
     FD_SET (fd, &wfds);
   if (ev & SEL_EXC)
     FD_SET (fd, &xfds);
+  errno = 0;
   return my_select (fd + 1, &rfds, &wfds, &xfds, timeout);
   /* In this case, when fd is invalid, on some platforms, the bit for fd
      is left alone in the fd_set, whereas on other platforms it is cleared.
@@ -426,6 +450,7 @@ test_function (select_fn my_select)
   test (test_tty, "TTY", my_select);
 #endif

+  result += test (test_bad_nfd, my_select, "Invalid nfd test");
   result += test (test_bad_fd, my_select, "Invalid fd test");
   result += test (test_connect_first, my_select, "Unconnected socket test");
   result += test (test_socket_pair, my_select, "Connected sockets test");
-- 
1.7.11.4