Source-Changes-HG archive

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

[src/trunk]: src/sys/kern Fix some screw cases in cmsg file descriptor passing.



details:   https://anonhg.NetBSD.org/src/rev/c23d1ac93506
branches:  trunk
changeset: 784913:c23d1ac93506
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Feb 14 01:00:07 2013 +0000

description:
Fix some screw cases in cmsg file descriptor passing.

- Don't leave garbage in the control buffer if allocating file
descriptors fails in unp_externalize.

- Scrub the space between CMSG_LEN and CMSG_SPACE to avoid kernel
memory disclosure in unp_externalize.

- Don't read past cmsg_len when closing file descriptors that
couldn't get delivered, in free_rights.

ok christos

diffstat:

 sys/kern/uipc_syscalls.c |  22 +++++++++++++---------
 sys/kern/uipc_usrreq.c   |  26 ++++++++++++++++++--------
 2 files changed, 31 insertions(+), 17 deletions(-)

diffs (96 lines):

diff -r 4aa2942116f3 -r c23d1ac93506 sys/kern/uipc_syscalls.c
--- a/sys/kern/uipc_syscalls.c  Thu Feb 14 00:55:25 2013 +0000
+++ b/sys/kern/uipc_syscalls.c  Thu Feb 14 01:00:07 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_syscalls.c,v 1.158 2012/12/29 18:51:39 mlelstv Exp $      */
+/*     $NetBSD: uipc_syscalls.c,v 1.159 2013/02/14 01:00:07 riastradh Exp $    */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uipc_syscalls.c,v 1.158 2012/12/29 18:51:39 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_syscalls.c,v 1.159 2013/02/14 01:00:07 riastradh Exp $");
 
 #include "opt_pipe.h"
 
@@ -809,17 +809,21 @@
 static void
 free_rights(struct mbuf *m)
 {
-       int nfd;
-       int i;
+       struct cmsghdr *cm;
        int *fdv;
+       unsigned int nfds, i;
+
+       KASSERT(sizeof(*cm) <= m->m_len);
+       cm = mtod(m, struct cmsghdr *);
 
-       nfd = m->m_len < CMSG_SPACE(sizeof(int)) ? 0
-           : (m->m_len - CMSG_SPACE(sizeof(int))) / sizeof(int) + 1;
-       fdv = (int *) CMSG_DATA(mtod(m,struct cmsghdr *));
-       for (i = 0; i < nfd; i++) {
+       KASSERT(CMSG_ALIGN(sizeof(*cm)) <= cm->cmsg_len);
+       KASSERT(cm->cmsg_len <= m->m_len);
+       nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) / sizeof(int);
+       fdv = (int *)CMSG_DATA(cm);
+
+       for (i = 0; i < nfds; i++)
                if (fd_getfile(fdv[i]) != NULL)
                        (void)fd_close(fdv[i]);
-       }
 }
 
 void
diff -r 4aa2942116f3 -r c23d1ac93506 sys/kern/uipc_usrreq.c
--- a/sys/kern/uipc_usrreq.c    Thu Feb 14 00:55:25 2013 +0000
+++ b/sys/kern/uipc_usrreq.c    Thu Feb 14 01:00:07 2013 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_usrreq.c,v 1.140 2012/10/06 22:58:08 christos Exp $       */
+/*     $NetBSD: uipc_usrreq.c,v 1.141 2013/02/14 01:00:07 riastradh 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.140 2012/10/06 22:58:08 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_usrreq.c,v 1.141 2013/02/14 01:00:07 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -1336,14 +1336,24 @@
        }
  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);
-               }
+               file_t **const fpp = (file_t **)CMSG_DATA(cm);
+               for (size_t i = 0; i < nfds; i++)
+                       unp_discard_now(fpp[i]);
+               /*
+                * Truncate the array so that nobody will try to interpret
+                * what is now garbage in it.
+                */
+               cm->cmsg_len = CMSG_LEN(0);
+               rights->m_len = CMSG_SPACE(0);
        }
 
+       /*
+        * Don't disclose kernel memory in the alignment space.
+        */
+       KASSERT(cm->cmsg_len <= rights->m_len);
+       memset(&mtod(rights, char *)[cm->cmsg_len], 0, rights->m_len -
+           cm->cmsg_len);
+
        rw_exit(&p->p_cwdi->cwdi_lock);
        kmem_free(fdp, nfds * sizeof(int));
        return error;



Home | Main Index | Thread Index | Old Index