Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/kern Bounding the 'nfds' arg to poll() at the current pr...



details:   https://anonhg.NetBSD.org/src/rev/1c9715a4f26c
branches:  trunk
changeset: 749915:1c9715a4f26c
user:      dsl <dsl%NetBSD.org@localhost>
date:      Sat Dec 12 17:47:05 2009 +0000

description:
Bounding the 'nfds' arg to poll() at the current process limit for actual
open files is rather gross - the poll map isn't required to be dense.
Instead limit to a much larger value (1000 + dt_nfiles) so that user
programs cannot allocate indefinite sized blocks of kvm.
If the limit is exceeded, then return EINVAL instead of silently truncating
the list.
(The silent truncation in select isn't quite as bad - although even there
any high bits that are set ought to generate an EBADF response.)
Move the code that converts ERESTART and EWOULDBLOCK into common code.
Effectively fixes PR/17507 since the new limit is unlikely to be detected.

diffstat:

 sys/kern/sys_select.c |  45 ++++++++++++++++++++++++---------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diffs (105 lines):

diff -r 2cc56282b933 -r 1c9715a4f26c sys/kern/sys_select.c
--- a/sys/kern/sys_select.c     Sat Dec 12 17:29:34 2009 +0000
+++ b/sys/kern/sys_select.c     Sat Dec 12 17:47:05 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sys_select.c,v 1.19 2009/11/11 09:48:51 rmind Exp $    */
+/*     $NetBSD: sys_select.c,v 1.20 2009/12/12 17:47:05 dsl Exp $      */
 
 /*-
  * Copyright (c) 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.19 2009/11/11 09:48:51 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.20 2009/12/12 17:47:05 dsl Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -261,6 +261,12 @@
                l->l_sigmask = oldmask;
                mutex_exit(p->p_lock);
        }
+
+       /* select and poll are not restarted after signals... */
+       if (error == ERESTART)
+               return EINTR;
+       if (error == EWOULDBLOCK)
+               return 0;
        return error;
 }
 
@@ -293,7 +299,7 @@
        if (u_ ## name) {                                               \
                error = copyin(u_ ## name, bits + ni * x, ni);          \
                if (error)                                              \
-                       goto done;                                      \
+                       goto fail;                                      \
        } else                                                          \
                memset(bits + ni * x, 0, ni);
        getbits(in, 0);
@@ -302,18 +308,13 @@
 #undef getbits
 
        error = sel_do_scan(bits, nd, ts, mask, retval, 1);
- done:
-       /* select is not restarted after signals... */
-       if (error == ERESTART)
-               error = EINTR;
-       if (error == EWOULDBLOCK)
-               error = 0;
        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);
+ fail:
        if (bits != smallbits)
                kmem_free(bits, ni * 6);
        return (error);
@@ -418,12 +419,19 @@
        struct pollfd   smallfds[32];
        struct pollfd   *fds;
        int             error;
-       size_t          ni, nf;
+       size_t          ni;
 
-       nf = curlwp->l_fd->fd_dt->dt_nfiles;
-       if (nfds > nf) {
-               /* forgiving; slightly wrong */
-               nfds = nf;
+       if (nfds > 1000 + curlwp->l_fd->fd_dt->dt_nfiles) {
+               /*
+                * Either the user passed in a very sparse 'fds' or junk!
+                * The kmem_alloc() call below would be bad news.
+                * We could process the 'fds' array in chunks, but that
+                * is a lot of code that isn't normally useful.
+                * (Or just move the copyin/out into pollscan().)
+                * Historically the code silently truncated 'fds' to
+                * dt_nfiles entries - but that does cause issues.
+                */
+               return EINVAL;
        }
        ni = nfds * sizeof(struct pollfd);
        if (ni > sizeof(smallfds)) {
@@ -435,17 +443,12 @@
 
        error = copyin(u_fds, fds, ni);
        if (error)
-               goto done;
+               goto fail;
 
        error = sel_do_scan(fds, nfds, ts, mask, retval, 0);
- done:
-       /* poll is not restarted after signals... */
-       if (error == ERESTART)
-               error = EINTR;
-       if (error == EWOULDBLOCK)
-               error = 0;
        if (error == 0)
                error = copyout(fds, u_fds, ni);
+ fail:
        if (fds != smallfds)
                kmem_free(fds, ni);
        return (error);



Home | Main Index | Thread Index | Old Index