Date: Wed, 12 Sep 2018 19:13:32 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r338618 - in stable/11/sys: compat/cloudabi compat/freebsd32 compat/linux kern sys Message-ID: <201809121913.w8CJDWre056371@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201809121913.w8CJDWre056371>