Source-Changes-HG archive

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

[src/netbsd-6]: src/sys/kern Pull up following revision(s) (requested by chri...



details:   https://anonhg.NetBSD.org/src/rev/e2a674ef84b3
branches:  netbsd-6
changeset: 774642:e2a674ef84b3
user:      riz <riz%NetBSD.org@localhost>
date:      Tue Oct 09 23:45:21 2012 +0000

description:
Pull up following revision(s) (requested by christos in ticket #593):
        sys/kern/uipc_usrreq.c: revision 1.140
Avoid crash dereferencing a NULL fp in fd_affix() in unp_externalize
caused by the sequence of passing two fd's with two sendmsg()'s,
then doing a read() and a recvmsg(). The read() calls dom_dispose()
which discards both messages in the mbuf, and sets the fp's in the
array to NULL. Linux dequeues only one message per read() so the
second recvmsg() gets the fd from the second message.  This fix
just avoids the NULL pointer de-reference, making the second
recvmsg() to fail. It is dubious to pass fd's with stream sockets
and expect mixing read() and recvmsg() to work. Plus processing
one control message per read() changes the current semantics and
should be examined before applied. In addition there is a race between
dom_externalize() and dom_dispose(): what happens in a multi-threaded
network stack when one thread disposes where the other externalizes
the same array?
NB: Pullup to 6.

diffstat:

 sys/kern/uipc_usrreq.c |  119 +++++++++++++++++++++++++-----------------------
 1 files changed, 61 insertions(+), 58 deletions(-)

diffs (186 lines):

diff -r 84ed57a7b58d -r e2a674ef84b3 sys/kern/uipc_usrreq.c
--- a/sys/kern/uipc_usrreq.c    Tue Oct 09 23:12:57 2012 +0000
+++ b/sys/kern/uipc_usrreq.c    Tue Oct 09 23:45:21 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_usrreq.c,v 1.136.8.1 2012/06/11 23:20:38 riz Exp $        */
+/*     $NetBSD: uipc_usrreq.c,v 1.136.8.2 2012/10/09 23:45:21 riz Exp $        */
 
 /*-
  * Copyright (c) 1998, 2000, 2004, 2008, 2009 The NetBSD Foundation, Inc.
@@ -96,7 +96,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uipc_usrreq.c,v 1.136.8.1 2012/06/11 23:20:38 riz Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_usrreq.c,v 1.136.8.2 2012/10/09 23:45:21 riz Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1235,78 +1235,66 @@
 int
 unp_externalize(struct mbuf *rights, struct lwp *l, int flags)
 {
-       struct cmsghdr *cm = mtod(rights, struct cmsghdr *);
-       struct proc *p = l->l_proc;
-       int i, *fdp;
+       struct cmsghdr * const cm = mtod(rights, struct cmsghdr *);
+       struct proc * const p = l->l_proc;
        file_t **rp;
-       file_t *fp;
-       int nfds, error = 0;
+       int error = 0;
 
-       nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) /
+       const size_t nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) /
            sizeof(file_t *);
-       rp = (file_t **)CMSG_DATA(cm);
 
-       fdp = malloc(nfds * sizeof(int), M_TEMP, M_WAITOK);
+       int * const fdp = kmem_alloc(nfds * sizeof(int), KM_SLEEP);
        rw_enter(&p->p_cwdi->cwdi_lock, RW_READER);
 
        /* Make sure the recipient should be able to see the files.. */
-       if (p->p_cwdi->cwdi_rdir != NULL) {
-               rp = (file_t **)CMSG_DATA(cm);
-               for (i = 0; i < nfds; i++) {
-                       fp = *rp++;
-                       /*
-                        * If we are in a chroot'ed directory, and
-                        * someone wants to pass us a directory, make
-                        * sure it's inside the subtree we're allowed
-                        * to access.
-                        */
-                       if (fp->f_type == DTYPE_VNODE) {
-                               vnode_t *vp = (vnode_t *)fp->f_data;
-                               if ((vp->v_type == VDIR) &&
-                                   !vn_isunder(vp, p->p_cwdi->cwdi_rdir, l)) {
-                                       error = EPERM;
-                                       break;
-                               }
+       rp = (file_t **)CMSG_DATA(cm);
+       for (size_t i = 0; i < nfds; i++) {
+               file_t * const fp = *rp++;
+               if (fp == NULL) {
+                       error = EINVAL;
+                       goto out;
+               }
+               /*
+                * If we are in a chroot'ed directory, and
+                * someone wants to pass us a directory, make
+                * sure it's inside the subtree we're allowed
+                * to access.
+                */
+               if (p->p_cwdi->cwdi_rdir != NULL && fp->f_type == DTYPE_VNODE) {
+                       vnode_t *vp = (vnode_t *)fp->f_data;
+                       if ((vp->v_type == VDIR) &&
+                           !vn_isunder(vp, p->p_cwdi->cwdi_rdir, l)) {
+                               error = EPERM;
+                               goto out;
                        }
                }
        }
 
  restart:
-       rp = (file_t **)CMSG_DATA(cm);
-       if (error != 0) {
-               for (i = 0; i < nfds; i++) {
-                       fp = *rp;
-                       *rp++ = 0;
-                       unp_discard_now(fp);
-               }
-               goto out;
-       }
-
        /*
         * First loop -- allocate file descriptor table slots for the
         * new files.
         */
-       for (i = 0; i < nfds; i++) {
-               fp = *rp++;
+       for (size_t i = 0; i < nfds; i++) {
                if ((error = fd_alloc(p, 0, &fdp[i])) != 0) {
                        /*
                         * Back out what we've done so far.
                         */
-                       for (--i; i >= 0; i--) {
+                       while (i-- > 0) {
                                fd_abort(p, NULL, fdp[i]);
                        }
                        if (error == ENOSPC) {
                                fd_tryexpand(p);
                                error = 0;
-                       } else {
-                               /*
-                                * This is the error that has historically
-                                * been returned, and some callers may
-                                * expect it.
-                                */
-                               error = EMSGSIZE;
+                               goto restart;
                        }
-                       goto restart;
+                       /*
+                        * This is the error that has historically
+                        * been returned, and some callers may
+                        * expect it.
+                        */
+                       error = EMSGSIZE;
+                       goto out;
                }
        }
 
@@ -1315,12 +1303,17 @@
         * file passing state and affix the descriptors.
         */
        rp = (file_t **)CMSG_DATA(cm);
-       for (i = 0; i < nfds; i++) {
-               int fd = fdp[i];
-               fp = *rp++;
+       int *ofdp = (int *)CMSG_DATA(cm);
+       for (size_t i = 0; i < nfds; i++) {
+               file_t * const fp = *rp++;
+               const int fd = fdp[i];
                atomic_dec_uint(&unp_rights);
                fd_set_exclose(l, fd, (flags & O_CLOEXEC) != 0);
                fd_affix(p, fp, fd);
+               /*
+                * Done with this file pointer, replace it with a fd;
+                */
+               *ofdp++ = fd;
                mutex_enter(&fp->f_lock);
                fp->f_msgcount--;
                mutex_exit(&fp->f_lock);
@@ -1334,16 +1327,26 @@
        }
 
        /*
-        * Copy temporary array to message and adjust length, in case of
-        * transition from large file_t pointers to ints.
+        * Adjust length, in case of transition from large file_t
+        * pointers to ints.
         */
-       memcpy(CMSG_DATA(cm), fdp, nfds * sizeof(int));
-       cm->cmsg_len = CMSG_LEN(nfds * sizeof(int));
-       rights->m_len = CMSG_SPACE(nfds * sizeof(int));
+       if (sizeof(file_t *) != sizeof(int)) {
+               cm->cmsg_len = CMSG_LEN(nfds * sizeof(int));
+               rights->m_len = CMSG_SPACE(nfds * sizeof(int));
+       }
  out:
+       if (__predict_false(error != 0)) {
+               rp = (file_t **)CMSG_DATA(cm);
+               for (size_t i = 0; i < nfds; i++) {
+                       file_t * const fp = *rp;
+                       *rp++ = 0;
+                       unp_discard_now(fp);
+               }
+       }
+
        rw_exit(&p->p_cwdi->cwdi_lock);
-       free(fdp, M_TEMP);
-       return (error);
+       kmem_free(fdp, nfds * sizeof(int));
+       return error;
 }
 
 int



Home | Main Index | Thread Index | Old Index