Module Name: src
Committed By: martin
Date: Mon Nov 18 18:16:52 UTC 2024
Modified Files:
src/sys/kern [netbsd-10]: sys_select.c
src/tests/lib/libc/sys [netbsd-10]: t_select.c
Log Message:
Pull up following revision(s) (requested by riastradh in ticket #1011):
sys/kern/sys_select.c: revision 1.67
tests/lib/libc/sys/t_select.c: revision 1.5
PR kern/57504 : Check all fds passed in to select
If an application passes in a huge fd_set (select(BIG, ...))
then check every bit in the fd_sets provided, to make sure
they are valid.
If BIG is too big (cannot possibly represent an open fd for
this process, under any circumstances: ie: not just because
that many are not currently open) return EINVAL.
Otherwise, check every set bit to make sure it is valid. Any
fd bits set above the applications current highest open fd
automatically generate EBADF and quick(ish) exit.
fd's that are within the plausible range are then checked as
they always were (it is possible for there to be a few there
above the max open fd - as everything in select is done in
multiples of __FDBITS (fd_mask) but the max open fd is not so
constrained. Those always were checked, continue using the
same mechanism.
This should have zero impact on any sane application which
uses the highest fd for which it set a bit, +1, as the first
arg to select. However, if there are any broken applications
that were relying upon the previous behaviour of simply ignoring
any fd_masks that started beyond the max number of open files,
then they might (if they happen to have any bits set) now fail.
tests/lib/libc/sys/t_select: Test select on bad file descriptors.
This should immediately fail, not hang, even if the bad fd is
high-numbered.
PR kern/57504: select with large enough bogus fd number set hangs
instead of failing with EBADF
To generate a diff of this commit:
cvs rdiff -u -r1.60 -r1.60.4.1 src/sys/kern/sys_select.c
cvs rdiff -u -r1.4 -r1.4.28.1 src/tests/lib/libc/sys/t_select.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/sys_select.c
diff -u src/sys/kern/sys_select.c:1.60 src/sys/kern/sys_select.c:1.60.4.1
--- src/sys/kern/sys_select.c:1.60 Wed Jun 29 22:27:01 2022
+++ src/sys/kern/sys_select.c Mon Nov 18 18:16:52 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: sys_select.c,v 1.60 2022/06/29 22:27:01 riastradh Exp $ */
+/* $NetBSD: sys_select.c,v 1.60.4.1 2024/11/18 18:16:52 martin Exp $ */
/*-
* Copyright (c) 2007, 2008, 2009, 2010, 2019, 2020 The NetBSD Foundation, Inc.
@@ -84,7 +84,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.60 2022/06/29 22:27:01 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.60.4.1 2024/11/18 18:16:52 martin Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -348,6 +348,29 @@ state_check:
return error;
}
+/* designed to be compatible with FD_SET() FD_ISSET() ... */
+static int
+anyset(void *p, size_t nbits)
+{
+ size_t nwords;
+ __fd_mask mask;
+ __fd_mask *f = (__fd_mask *)p;
+
+ nwords = nbits / __NFDBITS;
+
+ while (nwords-- > 0)
+ if (*f++ != 0)
+ return 1;
+
+ nbits &= __NFDMASK;
+ if (nbits != 0) {
+ mask = (1U << nbits) - 1;
+ if ((*f & mask) != 0)
+ return 1;
+ }
+ return 0;
+}
+
int
selcommon(register_t *retval, int nd, fd_set *u_in, fd_set *u_ou,
fd_set *u_ex, struct timespec *ts, sigset_t *mask)
@@ -355,41 +378,123 @@ selcommon(register_t *retval, int nd, fd
char smallbits[howmany(FD_SETSIZE, NFDBITS) *
sizeof(fd_mask) * 6];
char *bits;
- int error, nf;
+ int error, nf, fb, db;
size_t ni;
if (nd < 0)
- return (EINVAL);
+ return EINVAL;
+
nf = atomic_load_consume(&curlwp->l_fd->fd_dt)->dt_nfiles;
- if (nd > nf) {
- /* forgiving; slightly wrong */
- nd = nf;
+
+ /*
+ * Don't allow absurdly large numbers of fds to be selected.
+ * (used to silently truncate, naughty naughty, no more ...)
+ *
+ * The additional FD_SETSISE allows for cases where the limit
+ * is not a round binary number, but the fd_set wants to
+ * include all the possible fds, as fd_sets are always
+ * multiples of 32 bits (__NFDBITS extra would be enough).
+ *
+ * The first test handles the case where the res limit has been
+ * set lower after some fds were opened, we always allow selecting
+ * up to the highest currently open fd.
+ */
+ if (nd > nf + FD_SETSIZE &&
+ nd > curlwp->l_proc->p_rlimit[RLIMIT_NOFILE].rlim_max + FD_SETSIZE)
+ return EINVAL;
+
+ fb = howmany(nf, __NFDBITS); /* how many fd_masks */
+ db = howmany(nd, __NFDBITS);
+
+ if (db > fb) {
+ size_t off;
+
+ /*
+ * the application wants to supply more fd masks than can
+ * possibly represent valid file descriptors.
+ *
+ * Check the excess fd_masks, if any bits are set in them
+ * that must be an error (cannot represent valid fd).
+ *
+ * Supplying lots of extra cleared fd_masks is dumb,
+ * but harmless, so allow that.
+ */
+ ni = (db - fb) * sizeof(fd_mask); /* excess bytes */
+ bits = smallbits;
+
+ /* skip over the valid fd_masks, those will be checked below */
+ off = howmany(nf, __NFDBITS) * sizeof(__fd_mask);
+
+ nd -= fb * NFDBITS; /* the number of excess fds */
+
+#define checkbits(name, o, sz, fds) \
+ do { \
+ if (u_ ## name != NULL) { \
+ error = copyin((char *)u_ ## name + o, \
+ bits, sz); \
+ if (error) \
+ goto fail; \
+ if (anyset(bits, (fds) ? \
+ (size_t)(fds) : CHAR_BIT * (sz))) { \
+ error = EBADF; \
+ goto fail; \
+ } \
+ } \
+ } while (0)
+
+ while (ni > sizeof(smallbits)) {
+ checkbits(in, off, sizeof(smallbits), 0);
+ checkbits(ou, off, sizeof(smallbits), 0);
+ checkbits(ex, off, sizeof(smallbits), 0);
+
+ off += sizeof(smallbits);
+ ni -= sizeof(smallbits);
+ nd -= sizeof(smallbits) * CHAR_BIT;
+ }
+ checkbits(in, off, ni, nd);
+ checkbits(ou, off, ni, nd);
+ checkbits(ex, off, ni, nd);
+#undef checkbits
+
+ db = fb; /* now just check the plausible fds */
+ nd = db * __NFDBITS;
}
- ni = howmany(nd, NFDBITS) * sizeof(fd_mask);
+
+ ni = db * sizeof(fd_mask);
if (ni * 6 > sizeof(smallbits))
bits = kmem_alloc(ni * 6, KM_SLEEP);
else
bits = smallbits;
#define getbits(name, x) \
- if (u_ ## name) { \
- error = copyin(u_ ## name, bits + ni * x, ni); \
- if (error) \
- goto fail; \
- } else \
- memset(bits + ni * x, 0, ni);
+ do { \
+ if (u_ ## name) { \
+ error = copyin(u_ ## name, bits + ni * x, ni); \
+ if (error) \
+ goto fail; \
+ } else \
+ memset(bits + ni * x, 0, ni); \
+ } while (0)
+
getbits(in, 0);
getbits(ou, 1);
getbits(ex, 2);
#undef getbits
error = sel_do_scan(selop_select, bits, nd, ni, ts, mask, retval);
- if (error == 0 && u_in != NULL)
- error = copyout(bits + ni * 3, u_in, ni);
- if (error == 0 && u_ou != NULL)
- error = copyout(bits + ni * 4, u_ou, ni);
- if (error == 0 && u_ex != NULL)
- error = copyout(bits + ni * 5, u_ex, ni);
+
+#define copyback(name, x) \
+ do { \
+ if (error == 0 && u_ ## name != NULL) \
+ error = copyout(bits + ni * x, \
+ u_ ## name, ni); \
+ } while (0)
+
+ copyback(in, 3);
+ copyback(ou, 4);
+ copyback(ex, 5);
+#undef copyback
+
fail:
if (bits != smallbits)
kmem_free(bits, ni * 6);
Index: src/tests/lib/libc/sys/t_select.c
diff -u src/tests/lib/libc/sys/t_select.c:1.4 src/tests/lib/libc/sys/t_select.c:1.4.28.1
--- src/tests/lib/libc/sys/t_select.c:1.4 Fri Jan 13 21:18:33 2017
+++ src/tests/lib/libc/sys/t_select.c Mon Nov 18 18:16:52 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: t_select.c,v 1.4 2017/01/13 21:18:33 christos Exp $ */
+/* $NetBSD: t_select.c,v 1.4.28.1 2024/11/18 18:16:52 martin Exp $ */
/*-
* Copyright (c) 2011 The NetBSD Foundation, Inc.
@@ -29,18 +29,20 @@
* POSSIBILITY OF SUCH DAMAGE.
*/
-#include <assert.h>
#include <sys/types.h>
+
#include <sys/select.h>
#include <sys/wait.h>
+
+#include <assert.h>
#include <err.h>
-#include <stdio.h>
-#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
#include <signal.h>
+#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
#include <atf-c.h>
@@ -207,11 +209,42 @@ ATF_TC_BODY(pselect_timeout, tc)
}
}
+ATF_TC(select_badfd);
+ATF_TC_HEAD(select_badfd, tc)
+{
+
+ atf_tc_set_md_var(tc, "descr", "Checks select rejects bad fds");
+}
+
+ATF_TC_BODY(select_badfd, tc)
+{
+ int fd;
+
+ for (fd = 0; fd < FD_SETSIZE; fd++) {
+ fd_set readfds;
+ int ret, error;
+
+ if (fcntl(fd, F_GETFL) != -1 || errno != EBADF)
+ continue;
+
+ FD_ZERO(&readfds);
+ FD_SET(fd, &readfds);
+ alarm(5);
+ errno = 0;
+ ret = select(fd + 1, &readfds, NULL, NULL, NULL);
+ error = errno;
+ alarm(0);
+ errno = error;
+ ATF_CHECK_ERRNO(EBADF, ret == -1);
+ }
+}
+
ATF_TP_ADD_TCS(tp)
{
ATF_TP_ADD_TC(tp, pselect_sigmask);
ATF_TP_ADD_TC(tp, pselect_timeout);
+ ATF_TP_ADD_TC(tp, select_badfd);
return atf_no_error();
}
