d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve handling of control message truncation.

Browse Source 
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
This commit is contained in:
markj 2018-08-07 16:36:48 +00:00
parent 455de2f90f
commit 7a979485ab
6 changed files with 136 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
sys/compat/cloudabi/cloudabi_sock.c
View File

@ -120,24 +120,27 @@ cloudabi_sock_recv(struct thread *td, cloudabi_fd_t fd, struct iovec *data,
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

62
sys/compat/freebsd32/freebsd32_misc.c
View File

@ -1064,7 +1064,7 @@ freebsd32_copyoutmsghdr(struct msghdr *msg, struct msghdr32 *msg32)
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 *control)
{
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 *control)
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 *control)
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 *control)
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);
}

33
sys/compat/linux/linux_socket.c
View File

@ -1263,7 +1263,7 @@ linux_recvmsg_common(struct thread *td, l_int s, struct l_msghdr *msghdr,
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, struct l_msghdr *msghdr,
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, struct l_msghdr *msghdr,
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, struct l_msghdr *msghdr,
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, struct l_msghdr *msghdr,
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, struct l_msghdr *msghdr,
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);

88
sys/kern/uipc_syscalls.c
View File

@ -905,7 +905,7 @@ kern_recvit(struct thread *td, int s, struct msghdr *mp, enum uio_seg fromseg,
{
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 *mp, enum uio_seg fromseg,
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 *mp, enum uio_seg fromseg,
control->m_data += sizeof (struct cmsghdr);
}
#endif
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)
len = mp->msg_controllen;
mp->msg_controllen = 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;
}
if (m != NULL) {
mp->msg_flags |= MSG_CTRUNC;
m_dispose_extcontrolm(m);
}
mp->msg_controllen = ctlbuf - (caddr_t)mp->msg_control;
}
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);
}
@ -1573,3 +1569,51 @@ getsockaddr(struct sockaddr **namp, caddr_t uaddr, size_t len)
}
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);
}
}

7
sys/kern/uipc_usrreq.c
View File

@ -2050,6 +2050,13 @@ unp_externalize(struct mbuf *control, struct mbuf **controlp, int flags)
&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 {

4
sys/sys/mbuf.h
View File

@ -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,