svn commit: r337423 - in head/sys: compat/cloudabi compat/freebsd32 compat/linux kern sys
Mark Johnston
markj at FreeBSD.org
Tue Aug 7 16:36:51 UTC 2018
Author: markj
Date: Tue Aug 7 16:36:48 2018
New Revision: 337423
URL: https://svnweb.freebsd.org/changeset/base/337423
Log:
Improve handling of control message truncation.
If a recvmsg(2) or recvmmsg(2) caller doesn't provide sufficient space
for all control messages, the kernel sets MSG_CTRUNC in the message
flags to indicate truncation of the control messages. In the case
of SCM_RIGHTS messages, however, we were failing to dispose of the
rights that had already been externalized into the recipient's file
descriptor table. Add a new function and mbuf type to handle this
cleanup task, and use it any time we fail to copy control messages
out to the recipient. To simplify cleanup, control message truncation
is now only performed at control message boundaries.
The change also fixes a few related bugs:
- Rights could be leaked to the recipient process if an error occurred
while copying out a message's contents.
- We failed to set MSG_CTRUNC if the truncation occurred on a control
message boundary, e.g., if the caller received two control messages
and provided only the exact amount of buffer space needed for the
first.
PR: 131876
Reviewed by: ed (previous version)
MFC after: 1 month
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D16561
Modified:
head/sys/compat/cloudabi/cloudabi_sock.c
head/sys/compat/freebsd32/freebsd32_misc.c
head/sys/compat/linux/linux_socket.c
head/sys/kern/uipc_syscalls.c
head/sys/kern/uipc_usrreq.c
head/sys/sys/mbuf.h
Modified: head/sys/compat/cloudabi/cloudabi_sock.c
==============================================================================
--- head/sys/compat/cloudabi/cloudabi_sock.c Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/compat/cloudabi/cloudabi_sock.c Tue Aug 7 16:36:48 2018 (r337423)
@@ -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: head/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- head/sys/compat/freebsd32/freebsd32_misc.c Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/compat/freebsd32/freebsd32_misc.c Tue Aug 7 16:36:48 2018 (r337423)
@@ -1064,7 +1064,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 {
@@ -1122,7 +1122,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;
@@ -1133,15 +1133,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;
@@ -1152,34 +1148,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;
@@ -1190,20 +1188,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
@@ -1240,19 +1241,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: head/sys/compat/linux/linux_socket.c
==============================================================================
--- head/sys/compat/linux/linux_socket.c Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/compat/linux/linux_socket.c Tue Aug 7 16:36:48 2018 (r337423)
@@ -1263,7 +1263,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;
@@ -1322,9 +1322,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);
@@ -1332,15 +1331,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;
}
@@ -1348,8 +1347,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);
@@ -1394,14 +1392,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;
}
}
@@ -1422,15 +1419,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: head/sys/kern/uipc_syscalls.c
==============================================================================
--- head/sys/kern/uipc_syscalls.c Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/kern/uipc_syscalls.c Tue Aug 7 16:36:48 2018 (r337423)
@@ -905,7 +905,7 @@ kern_recvit(struct thread *td, int s, struct msghdr *m
{
struct uio auio;
struct iovec *iov;
- struct mbuf *m, *control = NULL;
+ struct mbuf *control, *m;
caddr_t ctlbuf;
struct file *fp;
struct socket *so;
@@ -952,6 +952,7 @@ kern_recvit(struct thread *td, int s, struct msghdr *m
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,
@@ -1015,30 +1016,22 @@ kern_recvit(struct thread *td, int s, struct msghdr *m
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);
@@ -1050,8 +1043,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);
}
@@ -1572,4 +1568,52 @@ getsockaddr(struct sockaddr **namp, caddr_t uaddr, siz
*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;
+ 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_no_rights,
+ &fp);
+ if (error == 0)
+ fdclose(td, fp, fd);
+ }
+ }
+ clen -= datalen;
+ cm = (struct cmsghdr *)((uint8_t *)cm + datalen);
+ }
+ m_chtype(m, MT_CONTROL);
+ }
}
Modified: head/sys/kern/uipc_usrreq.c
==============================================================================
--- head/sys/kern/uipc_usrreq.c Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/kern/uipc_usrreq.c Tue Aug 7 16:36:48 2018 (r337423)
@@ -2050,6 +2050,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: head/sys/sys/mbuf.h
==============================================================================
--- head/sys/sys/mbuf.h Tue Aug 7 15:24:19 2018 (r337422)
+++ head/sys/sys/mbuf.h Tue Aug 7 16:36:48 2018 (r337423)
@@ -568,7 +568,8 @@ struct mbuf {
#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_NOINIT 255 /* Not a type but a flag to allocate
a non-initialized mbuf */
@@ -633,6 +634,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 *, char *, u_int, m_ext_free_t,
More information about the svn-src-head
mailing list