[src/trunk]: src/sys/kern Avoid crash dereferencing a NULL fp in fd_affix() i...

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/83f0120570c4
branches:  trunk
changeset: 781919:83f0120570c4
user:      christos <christos%NetBSD.org@localhost>
date:      Sat Oct 06 22:58:08 2012 +0000

description:
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 |  41 +++++++++++++++++++++--------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diffs (67 lines):

diff -r beac25e727d3 -r 83f0120570c4 sys/kern/uipc_usrreq.c
--- a/sys/kern/uipc_usrreq.c    Sat Oct 06 22:15:09 2012 +0000
+++ b/sys/kern/uipc_usrreq.c    Sat Oct 06 22:58:08 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_usrreq.c,v 1.139 2012/07/30 10:45:03 christos Exp $       */
+/*     $NetBSD: uipc_usrreq.c,v 1.140 2012/10/06 22:58:08 christos 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.139 2012/07/30 10:45:03 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_usrreq.c,v 1.140 2012/10/06 22:58:08 christos Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1247,29 +1247,30 @@
        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 (size_t i = 0; i < nfds; i++) {
-                       file_t * const 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;
-                                       goto out;
-                               }
+       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);
        /*
         * First loop -- allocate file descriptor table slots for the
         * new files.