File descriptor passing internalizes fd's into an mbuf-stored array of

struct file *, and then externalizes back to fd upon delivery.
Convert storage to array of struct fdpass, containing struct file *
(and soon something else).  memcpy originally intended for alignment
purposes can be removed because CMSG_ALIGN is _ALIGN.

There is some anxiety over changing this code, but it reads easier.
ok mpi guenther kettenis

2 files changed, 44 insertions, 33 deletions

diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 94204dc527b..ef14e79c4c2 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_usrreq.c,v 1.109 2016/12/29 12:12:43 mpi Exp $	*/
+/*	$OpenBSD: uipc_usrreq.c,v 1.110 2017/01/23 23:44:04 deraadt Exp $	*/
 /*	$NetBSD: uipc_usrreq.c,v 1.18 1996/02/09 19:00:50 christos Exp $	*/
 
 /*
@@ -56,6 +56,10 @@ void	uipc_setaddr(const struct unpcb *, struct mbuf *);
 /* list of all UNIX domain sockets, for unp_gc() */
 LIST_HEAD(unp_head, unpcb) unp_head = LIST_HEAD_INITIALIZER(unp_head);
 
+struct fdpass {
+	struct file	*fp;
+};
+
 /*
  * Stack of sets of files that were passed over a socket but were
  * not received and need to be closed.
@@ -63,10 +67,15 @@ LIST_HEAD(unp_head, unpcb) unp_head = LIST_HEAD_INITIALIZER(unp_head);
 struct	unp_deferral {
 	SLIST_ENTRY(unp_deferral)	ud_link;
 	int	ud_n;
-	/* followed by ud_n struct file * pointers */
-	struct file *ud_fp[];
+	/* followed by ud_n struct fdpass */
+	struct fdpass ud_fp[];
 };
 
+void	unp_discard(struct fdpass *, int);
+void	unp_mark(struct fdpass *, int);
+void	unp_scan(struct mbuf *, void (*)(struct fdpass *, int));
+
+
 /* list of sets of files that were sent over sockets that are now closed */
 SLIST_HEAD(,unp_deferral) unp_deferred = SLIST_HEAD_INITIALIZER(unp_deferred);
 
@@ -664,12 +673,12 @@ unp_externalize(struct mbuf *rights, socklen_t controllen, int flags)
 	struct proc *p = curproc;		/* XXX */
 	struct cmsghdr *cm = mtod(rights, struct cmsghdr *);
 	int i, *fdp = NULL;
-	struct file **rp;
+	struct fdpass *rp;
 	struct file *fp;
 	int nfds, error = 0;
 
 	nfds = (cm->cmsg_len - CMSG_ALIGN(sizeof(*cm))) /
-	    sizeof(struct file *);
+	    sizeof(struct fdpass);
 	if (controllen < CMSG_ALIGN(sizeof(struct cmsghdr)))
 		controllen = 0;
 	else
@@ -680,9 +689,10 @@ unp_externalize(struct mbuf *rights, socklen_t controllen, int flags)
 	}
 
 	/* Make sure the recipient should be able to see the descriptors.. */
-	rp = (struct file **)CMSG_DATA(cm);
+	rp = (struct fdpass *)CMSG_DATA(cm);
 	for (i = 0; i < nfds; i++) {
-		fp = *rp++;
+		fp = rp->fp;
+		rp++;
 		error = pledge_recvfd(p, fp);
 		if (error)
 			break;
@@ -709,7 +719,7 @@ restart:
 	fdplock(p->p_fd);
 	if (error != 0) {
 		if (nfds > 0) {
-			rp = ((struct file **)CMSG_DATA(cm));
+			rp = ((struct fdpass *)CMSG_DATA(cm));
 			unp_discard(rp, nfds);
 		}
 		goto out;
@@ -719,7 +729,7 @@ restart:
 	 * First loop -- allocate file descriptor table slots for the
 	 * new descriptors.
 	 */
-	rp = ((struct file **)CMSG_DATA(cm));
+	rp = ((struct fdpass *)CMSG_DATA(cm));
 	for (i = 0; i < nfds; i++) {
 		if ((error = fdalloc(p, 0, &fdp[i])) != 0) {
 			/*
@@ -748,7 +758,8 @@ restart:
 		 * fdalloc() works properly.. We finalize it all
 		 * in the loop below.
 		 */
-		p->p_fd->fd_ofiles[fdp[i]] = *rp++;
+		p->p_fd->fd_ofiles[fdp[i]] = rp->fp;
+		rp++;
 
 		if (flags & MSG_CMSG_CLOEXEC)
 			p->p_fd->fd_ofileflags[fdp[i]] |= UF_EXCLOSE;
@@ -758,11 +769,12 @@ restart:
 	 * Now that adding them has succeeded, update all of the
 	 * descriptor passing state.
 	 */
-	rp = (struct file **)CMSG_DATA(cm);
+	rp = (struct fdpass *)CMSG_DATA(cm);
 	for (i = 0; i < nfds; i++) {
 		struct unpcb *unp;
 
-		fp = *rp++;
+		fp = rp->fp;
+		rp++;
 		if ((unp = fptounp(fp)) != NULL)
 			unp->unp_msgcount--;
 		unp_rights--;
@@ -787,7 +799,8 @@ unp_internalize(struct mbuf *control, struct proc *p)
 {
 	struct filedesc *fdp = p->p_fd;
 	struct cmsghdr *cm = mtod(control, struct cmsghdr *);
-	struct file **rp, *fp;
+	struct fdpass *rp;
+	struct file *fp;
 	struct unpcb *unp;
 	int i, error;
 	int nfds, *ip, fd, neededspace;
@@ -807,7 +820,7 @@ unp_internalize(struct mbuf *control, struct proc *p)
 
 	/* Make sure we have room for the struct file pointers */
 morespace:
-	neededspace = CMSG_SPACE(nfds * sizeof(struct file *)) -
+	neededspace = CMSG_SPACE(nfds * sizeof(struct fdpass)) -
 	    control->m_len;
 	if (neededspace > M_TRAILINGSPACE(control)) {
 		char *tmp;
@@ -834,11 +847,11 @@ morespace:
 	}
 
 	/* adjust message & mbuf to note amount of space actually used. */
-	cm->cmsg_len = CMSG_LEN(nfds * sizeof(struct file *));
-	control->m_len = CMSG_SPACE(nfds * sizeof(struct file *));
+	cm->cmsg_len = CMSG_LEN(nfds * sizeof(struct fdpass));
+	control->m_len = CMSG_SPACE(nfds * sizeof(struct fdpass));
 
 	ip = ((int *)CMSG_DATA(cm)) + nfds - 1;
-	rp = ((struct file **)CMSG_DATA(cm)) + nfds - 1;
+	rp = ((struct fdpass *)CMSG_DATA(cm)) + nfds - 1;
 	for (i = 0; i < nfds; i++) {
 		memcpy(&fd, ip, sizeof fd);
 		ip--;
@@ -859,7 +872,7 @@ morespace:
 			error = EINVAL;
 			goto fail;
 		}
-		memcpy(rp, &fp, sizeof fp);
+		rp->fp = fp;
 		rp--;
 		fp->f_count++;
 		if ((unp = fptounp(fp)) != NULL) {
@@ -873,7 +886,7 @@ fail:
 	/* Back out what we just did. */
 	for ( ; i > 0; i--) {
 		rp++;
-		memcpy(&fp, rp, sizeof(fp));
+		fp = rp->fp;
 		fp->f_count--;
 		if ((unp = fptounp(fp)) != NULL)
 			unp->unp_msgcount--;
@@ -902,7 +915,7 @@ unp_gc(void *arg __unused)
 	while ((defer = SLIST_FIRST(&unp_deferred)) != NULL) {
 		SLIST_REMOVE_HEAD(&unp_deferred, ud_link);
 		for (i = 0; i < defer->ud_n; i++) {
-			fp = defer->ud_fp[i];
+			fp = defer->ud_fp[i].fp;
 			if (fp == NULL)
 				continue;
 			FREF(fp);
@@ -911,7 +924,8 @@ unp_gc(void *arg __unused)
 			unp_rights--;
 			(void) closef(fp, NULL);
 		}
-		free(defer, M_TEMP, sizeof(*defer) + sizeof(fp) * defer->ud_n);
+		free(defer, M_TEMP, sizeof(*defer) +
+		    sizeof(struct fdpass) * defer->ud_n);
 	}
 
 	unp_defer = 0;
@@ -1005,10 +1019,10 @@ unp_dispose(struct mbuf *m)
 }
 
 void
-unp_scan(struct mbuf *m0, void (*op)(struct file **, int))
+unp_scan(struct mbuf *m0, void (*op)(struct fdpass *, int))
 {
 	struct mbuf *m;
-	struct file **rp;
+	struct fdpass *rp;
 	struct cmsghdr *cm;
 	int qfds;
 
@@ -1021,9 +1035,9 @@ unp_scan(struct mbuf *m0, void (*op)(struct file **, int))
 				    cm->cmsg_type != SCM_RIGHTS)
 					continue;
 				qfds = (cm->cmsg_len - CMSG_ALIGN(sizeof *cm))
-				    / sizeof(struct file *);
+				    / sizeof(struct fdpass);
 				if (qfds > 0) {
-					rp = (struct file **)CMSG_DATA(cm);
+					rp = (struct fdpass *)CMSG_DATA(cm);
 					op(rp, qfds);
 				}
 				break;		/* XXX, but saves time */
@@ -1034,16 +1048,16 @@ unp_scan(struct mbuf *m0, void (*op)(struct file **, int))
 }
 
 void
-unp_mark(struct file **rp, int nfds)
+unp_mark(struct fdpass *rp, int nfds)
 {
 	struct unpcb *unp;
 	int i;
 
 	for (i = 0; i < nfds; i++) {
-		if (rp[i] == NULL)
+		if (rp[i].fp == NULL)
 			continue;
 
-		unp = fptounp(rp[i]);
+		unp = fptounp(rp[i].fp);
 		if (unp == NULL)
 			continue;
 
@@ -1057,7 +1071,7 @@ unp_mark(struct file **rp, int nfds)
 }
 
 void
-unp_discard(struct file **rp, int nfds)
+unp_discard(struct fdpass *rp, int nfds)
 {
 	struct unp_deferral *defer;
 
diff --git a/sys/sys/unpcb.h b/sys/sys/unpcb.h
index ff1f134b7db..b4488e0ebe2 100644
--- a/sys/sys/unpcb.h
+++ b/sys/sys/unpcb.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: unpcb.h,v 1.12 2015/08/28 04:38:47 guenther Exp $	*/
+/*	$OpenBSD: unpcb.h,v 1.13 2017/01/23 23:44:04 deraadt Exp $	*/
 /*	$NetBSD: unpcb.h,v 1.6 1994/06/29 06:46:08 cgd Exp $	*/
 
 /*
@@ -91,12 +91,9 @@ int	unp_bind(struct unpcb *, struct mbuf *, struct proc *);
 int	unp_connect(struct socket *, struct mbuf *, struct proc *);
 int	unp_connect2(struct socket *, struct socket *);
 void	unp_detach(struct unpcb *);
-void	unp_discard(struct file **, int);
 void	unp_disconnect(struct unpcb *);
 void	unp_drop(struct unpcb *, int);
 void	unp_gc(void *);
-void	unp_mark(struct file **, int);
-void	unp_scan(struct mbuf *, void (*)(struct file **, int));
 void	unp_shutdown(struct unpcb *);
 int 	unp_externalize(struct mbuf *, socklen_t, int);
 int	unp_internalize(struct mbuf *, struct proc *);