svn commit: r338618 - in stable/11/sys: compat/cloudabi compat/freebsd32 compat/linux kern sys

Mark Johnston markj at FreeBSD.org
Wed Sep 12 19:13:34 UTC 2018


Author: markj
Date: Wed Sep 12 19:13:32 2018
New Revision: 338618
URL: https://svnweb.freebsd.org/changeset/base/338618

Log:
  MFC r337423:
  Improve handling of control message truncation.
  
  PR:	131876

Modified:
  stable/11/sys/compat/cloudabi/cloudabi_sock.c
  stable/11/sys/compat/freebsd32/freebsd32_misc.c
  stable/11/sys/compat/linux/linux_socket.c
  stable/11/sys/kern/uipc_syscalls.c
  stable/11/sys/kern/uipc_usrreq.c
  stable/11/sys/sys/mbuf.h
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/compat/cloudabi/cloudabi_sock.c
==============================================================================
--- stable/11/sys/compat/cloudabi/cloudabi_sock.c	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/compat/cloudabi/cloudabi_sock.c	Wed Sep 12 19:13:32 2018	(r338618)
@@ -120,24 +120,27 @@ cloudabi_sock_recv(struct thread *td, cloudabi_fd_t fd
 				    sizeof(int);
 				if (nfds > fdslen) {
 					/* Unable to store file descriptors. */
-					nfds = fdslen;
 					*rflags |=
 					    CLOUDABI_SOCK_RECV_FDS_TRUNCATED;
+					m_dispose_extcontrolm(control);
+					break;
 				}
 				error = copyout(CMSG_DATA(chdr), fds,
 				    nfds * sizeof(int));
-				if (error != 0) {
-					m_free(control);
-					return (error);
-				}
+				if (error != 0)
+					break;
 				fds += nfds;
 				fdslen -= nfds;
 				*rfdslen += nfds;
 			}
 		}
-		m_free(control);
+		if (control != NULL) {
+			if (error != 0)
+				m_dispose_extcontrolm(control);
+			m_free(control);
+		}
 	}
-	return (0);
+	return (error);
 }
 
 int

Modified: stable/11/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- stable/11/sys/compat/freebsd32/freebsd32_misc.c	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/compat/freebsd32/freebsd32_misc.c	Wed Sep 12 19:13:32 2018	(r338618)
@@ -907,7 +907,7 @@ freebsd32_copyoutmsghdr(struct msghdr *msg, struct msg
 				 FREEBSD32_ALIGN(sizeof(struct cmsghdr)))
 
 static size_t
-freebsd32_cmsg_convert(struct cmsghdr *cm, void *data, socklen_t datalen)
+freebsd32_cmsg_convert(const struct cmsghdr *cm, void *data, socklen_t datalen)
 {
 	size_t copylen;
 	union {
@@ -965,7 +965,7 @@ freebsd32_copy_msg_out(struct msghdr *msg, struct mbuf
 {
 	struct cmsghdr *cm;
 	void *data;
-	socklen_t clen, datalen, datalen_out;
+	socklen_t clen, datalen, datalen_out, oldclen;
 	int error;
 	caddr_t ctlbuf;
 	int len, maxlen, copylen;
@@ -976,15 +976,11 @@ freebsd32_copy_msg_out(struct msghdr *msg, struct mbuf
 	maxlen = msg->msg_controllen;
 	msg->msg_controllen = 0;
 
-	m = control;
 	ctlbuf = msg->msg_control;
-      
-	while (m && len > 0) {
+	for (m = control; m != NULL && len > 0; m = m->m_next) {
 		cm = mtod(m, struct cmsghdr *);
 		clen = m->m_len;
-
 		while (cm != NULL) {
-
 			if (sizeof(struct cmsghdr) > clen ||
 			    cm->cmsg_len > clen) {
 				error = EINVAL;
@@ -995,34 +991,36 @@ freebsd32_copy_msg_out(struct msghdr *msg, struct mbuf
 			datalen = (caddr_t)cm + cm->cmsg_len - (caddr_t)data;
 			datalen_out = freebsd32_cmsg_convert(cm, data, datalen);
 
-			/* Adjust message length */
-			cm->cmsg_len = FREEBSD32_ALIGN(sizeof(struct cmsghdr)) +
-			    datalen_out;
-
-			/* Copy cmsghdr */
+			/*
+			 * Copy out the message header.  Preserve the native
+			 * message size in case we need to inspect the message
+			 * contents later.
+			 */
 			copylen = sizeof(struct cmsghdr);
 			if (len < copylen) {
 				msg->msg_flags |= MSG_CTRUNC;
-				copylen = len;
+				m_dispose_extcontrolm(m);
+				goto exit;
 			}
-
+			oldclen = cm->cmsg_len;
+			cm->cmsg_len = FREEBSD32_ALIGN(sizeof(struct cmsghdr)) +
+			    datalen_out;
 			error = copyout(cm, ctlbuf, copylen);
-			if (error)
+			cm->cmsg_len = oldclen;
+			if (error != 0)
 				goto exit;
 
 			ctlbuf += FREEBSD32_ALIGN(copylen);
 			len    -= FREEBSD32_ALIGN(copylen);
 
-			if (len <= 0)
-				break;
-
-			/* Copy data */
 			copylen = datalen_out;
 			if (len < copylen) {
 				msg->msg_flags |= MSG_CTRUNC;
-				copylen = len;
+				m_dispose_extcontrolm(m);
+				break;
 			}
 
+			/* Copy out the message data. */
 			error = copyout(data, ctlbuf, copylen);
 			if (error)
 				goto exit;
@@ -1033,20 +1031,23 @@ freebsd32_copy_msg_out(struct msghdr *msg, struct mbuf
 			if (CMSG_SPACE(datalen) < clen) {
 				clen -= CMSG_SPACE(datalen);
 				cm = (struct cmsghdr *)
-					((caddr_t)cm + CMSG_SPACE(datalen));
+				    ((caddr_t)cm + CMSG_SPACE(datalen));
 			} else {
 				clen = 0;
 				cm = NULL;
 			}
-		}	
-		m = m->m_next;
+
+			msg->msg_controllen += FREEBSD32_ALIGN(sizeof(*cm)) +
+			    datalen_out;
+		}
 	}
+	if (len == 0 && m != NULL) {
+		msg->msg_flags |= MSG_CTRUNC;
+		m_dispose_extcontrolm(m);
+	}
 
-	msg->msg_controllen = (len <= 0) ? maxlen :  ctlbuf - (caddr_t)msg->msg_control;
-	
 exit:
 	return (error);
-
 }
 
 int
@@ -1083,19 +1084,22 @@ freebsd32_recvmsg(td, uap)
 	error = kern_recvit(td, uap->s, &msg, UIO_USERSPACE, controlp);
 	if (error == 0) {
 		msg.msg_iov = uiov;
-		
+
 		if (control != NULL)
 			error = freebsd32_copy_msg_out(&msg, control);
 		else
 			msg.msg_controllen = 0;
-		
+
 		if (error == 0)
 			error = freebsd32_copyoutmsghdr(&msg, uap->msg);
 	}
 	free(iov, M_IOV);
 
-	if (control != NULL)
+	if (control != NULL) {
+		if (error != 0)
+			m_dispose_extcontrolm(control);
 		m_freem(control);
+	}
 
 	return (error);
 }

Modified: stable/11/sys/compat/linux/linux_socket.c
==============================================================================
--- stable/11/sys/compat/linux/linux_socket.c	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/compat/linux/linux_socket.c	Wed Sep 12 19:13:32 2018	(r338618)
@@ -1266,7 +1266,7 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 	struct cmsgcred *cmcred;
 	struct l_cmsghdr *linux_cmsg = NULL;
 	struct l_ucred linux_ucred;
-	socklen_t datalen, outlen;
+	socklen_t datalen, maxlen, outlen;
 	struct l_msghdr linux_msg;
 	struct iovec *iov, *uiov;
 	struct mbuf *control = NULL;
@@ -1325,9 +1325,8 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 			goto bad;
 	}
 
-	outbuf = PTRIN(linux_msg.msg_control);
-	outlen = 0;
-
+	maxlen = linux_msg.msg_controllen;
+	linux_msg.msg_controllen = 0;
 	if (control) {
 		linux_cmsg = malloc(L_CMSG_HDRSZ, M_LINUX, M_WAITOK | M_ZERO);
 
@@ -1335,15 +1334,15 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 		msg->msg_controllen = control->m_len;
 
 		cm = CMSG_FIRSTHDR(msg);
-
+		outbuf = PTRIN(linux_msg.msg_control);
+		outlen = 0;
 		while (cm != NULL) {
 			linux_cmsg->cmsg_type =
 			    bsd_to_linux_cmsg_type(cm->cmsg_type);
 			linux_cmsg->cmsg_level =
 			    bsd_to_linux_sockopt_level(cm->cmsg_level);
-			if (linux_cmsg->cmsg_type == -1
-			    || cm->cmsg_level != SOL_SOCKET)
-			{
+			if (linux_cmsg->cmsg_type == -1 ||
+			    cm->cmsg_level != SOL_SOCKET) {
 				error = EINVAL;
 				goto bad;
 			}
@@ -1351,8 +1350,7 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 			data = CMSG_DATA(cm);
 			datalen = (caddr_t)cm + cm->cmsg_len - (caddr_t)data;
 
-			switch (cm->cmsg_type)
-			{
+			switch (cm->cmsg_type) {
 			case SCM_RIGHTS:
 				if (flags & LINUX_MSG_CMSG_CLOEXEC) {
 					fds = datalen / sizeof(int);
@@ -1397,14 +1395,13 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 				break;
 			}
 
-			if (outlen + LINUX_CMSG_LEN(datalen) >
-			    linux_msg.msg_controllen) {
+			if (outlen + LINUX_CMSG_LEN(datalen) > maxlen) {
 				if (outlen == 0) {
 					error = EMSGSIZE;
 					goto bad;
 				} else {
-					linux_msg.msg_flags |=
-					    LINUX_MSG_CTRUNC;
+					linux_msg.msg_flags |= LINUX_MSG_CTRUNC;
+					m_dispose_extcontrolm(control);
 					goto out;
 				}
 			}
@@ -1425,15 +1422,19 @@ linux_recvmsg_common(struct thread *td, l_int s, struc
 
 			cm = CMSG_NXTHDR(msg, cm);
 		}
+		linux_msg.msg_controllen = outlen;
 	}
 
 out:
-	linux_msg.msg_controllen = outlen;
 	error = copyout(&linux_msg, msghdr, sizeof(linux_msg));
 
 bad:
+	if (control != NULL) {
+		if (error != 0)
+			m_dispose_extcontrolm(control);
+		m_freem(control);
+	}
 	free(iov, M_IOV);
-	m_freem(control);
 	free(linux_cmsg, M_LINUX);
 
 	return (error);

Modified: stable/11/sys/kern/uipc_syscalls.c
==============================================================================
--- stable/11/sys/kern/uipc_syscalls.c	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/kern/uipc_syscalls.c	Wed Sep 12 19:13:32 2018	(r338618)
@@ -1014,7 +1014,7 @@ kern_recvit(td, s, mp, fromseg, controlp)
 {
 	struct uio auio;
 	struct iovec *iov;
-	struct mbuf *m, *control = NULL;
+	struct mbuf *control, *m;
 	caddr_t ctlbuf;
 	struct file *fp;
 	struct socket *so;
@@ -1062,6 +1062,7 @@ kern_recvit(td, s, mp, fromseg, controlp)
 	if (KTRPOINT(td, KTR_GENIO))
 		ktruio = cloneuio(&auio);
 #endif
+	control = NULL;
 	len = auio.uio_resid;
 	error = soreceive(so, &fromsa, &auio, NULL,
 	    (mp->msg_control || controlp) ? &control : NULL,
@@ -1125,30 +1126,22 @@ kern_recvit(td, s, mp, fromseg, controlp)
 			control->m_data += sizeof (struct cmsghdr);
 		}
 #endif
+		ctlbuf = mp->msg_control;
 		len = mp->msg_controllen;
-		m = control;
 		mp->msg_controllen = 0;
-		ctlbuf = mp->msg_control;
-
-		while (m && len > 0) {
-			unsigned int tocopy;
-
-			if (len >= m->m_len)
-				tocopy = m->m_len;
-			else {
-				mp->msg_flags |= MSG_CTRUNC;
-				tocopy = len;
-			}
-
-			if ((error = copyout(mtod(m, caddr_t),
-					ctlbuf, tocopy)) != 0)
+		for (m = control; m != NULL && len >= m->m_len; m = m->m_next) {
+			if ((error = copyout(mtod(m, caddr_t), ctlbuf,
+			    m->m_len)) != 0)
 				goto out;
 
-			ctlbuf += tocopy;
-			len -= tocopy;
-			m = m->m_next;
+			ctlbuf += m->m_len;
+			len -= m->m_len;
+			mp->msg_controllen += m->m_len;
 		}
-		mp->msg_controllen = ctlbuf - (caddr_t)mp->msg_control;
+		if (m != NULL) {
+			mp->msg_flags |= MSG_CTRUNC;
+			m_dispose_extcontrolm(m);
+		}
 	}
 out:
 	fdrop(fp, td);
@@ -1160,8 +1153,11 @@ out:
 
 	if (error == 0 && controlp != NULL)
 		*controlp = control;
-	else  if (control)
+	else if (control != NULL) {
+		if (error != 0)
+			m_dispose_extcontrolm(control);
 		m_freem(control);
+	}
 
 	return (error);
 }
@@ -1784,4 +1780,53 @@ getsockaddr(namp, uaddr, len)
 		*namp = sa;
 	}
 	return (error);
+}
+
+/*
+ * Dispose of externalized rights from an SCM_RIGHTS message.  This function
+ * should be used in error or truncation cases to avoid leaking file descriptors
+ * into the recipient's (the current thread's) table.
+ */
+void
+m_dispose_extcontrolm(struct mbuf *m)
+{
+	struct cmsghdr *cm;
+	struct file *fp;
+	struct thread *td;
+	cap_rights_t rights;
+	socklen_t clen, datalen;
+	int error, fd, *fds, nfd;
+
+	td = curthread;
+	for (; m != NULL; m = m->m_next) {
+		if (m->m_type != MT_EXTCONTROL)
+			continue;
+		cm = mtod(m, struct cmsghdr *);
+		clen = m->m_len;
+		while (clen > 0) {
+			if (clen < sizeof(*cm))
+				panic("%s: truncated mbuf %p", __func__, m);
+			datalen = CMSG_SPACE(cm->cmsg_len - CMSG_SPACE(0));
+			if (clen < datalen)
+				panic("%s: truncated mbuf %p", __func__, m);
+
+			if (cm->cmsg_level == SOL_SOCKET &&
+			    cm->cmsg_type == SCM_RIGHTS) {
+				fds = (int *)CMSG_DATA(cm);
+				nfd = (cm->cmsg_len - CMSG_SPACE(0)) /
+				    sizeof(int);
+
+				while (nfd-- > 0) {
+					fd = *fds++;
+					error = fget(td, fd,
+					    cap_rights_init(&rights), &fp);
+					if (error == 0)
+						fdclose(td, fp, fd);
+				}
+			}
+			clen -= datalen;
+			cm = (struct cmsghdr *)((uint8_t *)cm + datalen);
+		}
+		m_chtype(m, MT_CONTROL);
+	}
 }

Modified: stable/11/sys/kern/uipc_usrreq.c
==============================================================================
--- stable/11/sys/kern/uipc_usrreq.c	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/kern/uipc_usrreq.c	Wed Sep 12 19:13:32 2018	(r338618)
@@ -1829,6 +1829,13 @@ unp_externalize(struct mbuf *control, struct mbuf **co
 				    &fdep[i]->fde_caps);
 				unp_externalize_fp(fdep[i]->fde_file);
 			}
+
+			/*
+			 * The new type indicates that the mbuf data refers to
+			 * kernel resources that may need to be released before
+			 * the mbuf is freed.
+			 */
+			m_chtype(*controlp, MT_EXTCONTROL);
 			FILEDESC_XUNLOCK(fdesc);
 			free(fdep[0], M_FILECAPS);
 		} else {

Modified: stable/11/sys/sys/mbuf.h
==============================================================================
--- stable/11/sys/sys/mbuf.h	Wed Sep 12 18:52:18 2018	(r338617)
+++ stable/11/sys/sys/mbuf.h	Wed Sep 12 19:13:32 2018	(r338618)
@@ -523,7 +523,8 @@ void sf_ext_free_nocache(void *, void *);
 #define	MT_EXP4		12	/* for experimental use */
 
 #define	MT_CONTROL	14	/* extra-data protocol message */
-#define	MT_OOBDATA	15	/* expedited data  */
+#define	MT_EXTCONTROL	15	/* control message with externalized contents */
+#define	MT_OOBDATA	16	/* expedited data  */
 #define	MT_NTYPES	16	/* number of mbuf types for mbtypes[] */
 
 #define	MT_NOINIT	255	/* Not a type but a flag to allocate
@@ -589,6 +590,7 @@ void		 m_demote_pkthdr(struct mbuf *);
 void		 m_demote(struct mbuf *, int, int);
 struct mbuf	*m_devget(char *, int, int, struct ifnet *,
 		    void (*)(char *, caddr_t, u_int));
+void		 m_dispose_extcontrolm(struct mbuf *m);
 struct mbuf	*m_dup(const struct mbuf *, int);
 int		 m_dup_pkthdr(struct mbuf *, const struct mbuf *, int);
 void		 m_extadd(struct mbuf *, caddr_t, u_int,


More information about the svn-src-all mailing list