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 <ebl...@redhat.com> + 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. +@item +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