Module Name: src
Committed By: riz
Date: Tue Oct 9 23:45:21 UTC 2012
Modified Files:
src/sys/kern [netbsd-6]: uipc_usrreq.c
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.136.8.1 -r1.136.8.2 src/sys/kern/uipc_usrreq.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/uipc_usrreq.c
diff -u src/sys/kern/uipc_usrreq.c:1.136.8.1 src/sys/kern/uipc_usrreq.c:1.136.8.2
--- src/sys/kern/uipc_usrreq.c:1.136.8.1 Mon Jun 11 23:20:38 2012
+++ src/sys/kern/uipc_usrreq.c Tue Oct 9 23:45:21 2012
@@ -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 @@ unp_drain(void)
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 @@ unp_externalize(struct mbuf *rights, str
* 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 @@ unp_externalize(struct mbuf *rights, str
}
/*
- * 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
