summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMartin Pieuchot <mpi@cvs.openbsd.org>2017-01-24 05:44:10 +0000
committerMartin Pieuchot <mpi@cvs.openbsd.org>2017-01-24 05:44:10 +0000
commite1fe1b8639d92bbcd2966b7f7c4872ba145561db (patch)
tree4e2045f6728ca994e643947d9f1ef128e72bbee9
parentdf408d31a55d4a1ba2549af27cd554ef2c82dd0c (diff)
In accept(2) and accept4(2) allocate a new file descriptor before
checking if the socket head's queue is empty and possibly sleeping. This way we avoid lock ordering problems as the NET_LOCK() and fdplock() won't be held at the same time. Note that socketpair(2) and close(2) are the two remaining syscalls holding these locks at the same time. They both respect the same order: fdplock() then NET_LOCK(). Initial deadlock reported by kettenis@ and ajacoutot@. ok bluhm@, guenther@, deraadt@
-rw-r--r--sys/kern/uipc_syscalls.c71
1 files changed, 30 insertions, 41 deletions
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index ed24be8ab85..3efaa23ac0e 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: uipc_syscalls.c,v 1.144 2016/12/29 12:12:43 mpi Exp $ */
+/* $OpenBSD: uipc_syscalls.c,v 1.145 2017/01/24 05:44:09 mpi Exp $ */
/* $NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $ */
/*
@@ -265,7 +265,7 @@ doaccept(struct proc *p, int sock, struct sockaddr *name, socklen_t *anamelen,
{
struct filedesc *fdp = p->p_fd;
struct file *fp, *headfp;
- struct mbuf *nam;
+ struct mbuf *nam = NULL;
socklen_t namelen;
int error, s, tmpfd;
struct socket *head, *so;
@@ -277,19 +277,30 @@ doaccept(struct proc *p, int sock, struct sockaddr *name, socklen_t *anamelen,
return (error);
headfp = fp;
+
+ fdplock(fdp);
+ error = falloc(p, &fp, &tmpfd);
+ if (!error && (flags & SOCK_CLOEXEC))
+ fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
+ fdpunlock(fdp);
+ if (error) {
+ FRELE(headfp, p);
+ return (error);
+ }
+
redo:
NET_LOCK(s);
head = headfp->f_data;
if (isdnssocket(head) || (head->so_options & SO_ACCEPTCONN) == 0) {
error = EINVAL;
- goto bad;
+ goto out;
}
if ((head->so_state & SS_NBIO) && head->so_qlen == 0) {
if (head->so_state & SS_CANTRCVMORE)
error = ECONNABORTED;
else
error = EWOULDBLOCK;
- goto bad;
+ goto out;
}
while (head->so_qlen == 0 && head->so_error == 0) {
if (head->so_state & SS_CANTRCVMORE) {
@@ -298,48 +309,30 @@ redo:
}
error = tsleep(&head->so_timeo, PSOCK | PCATCH,
"netcon", 0);
- if (error) {
- goto bad;
- }
+ if (error)
+ goto out;
}
if (head->so_error) {
error = head->so_error;
head->so_error = 0;
- goto bad;
+ goto out;
}
/* Figure out whether the new socket should be non-blocking. */
nflag = flags & SOCK_NONBLOCK_INHERIT ? (headfp->f_flag & FNONBLOCK)
: (flags & SOCK_NONBLOCK ? FNONBLOCK : 0);
- fdplock(fdp);
- error = falloc(p, &fp, &tmpfd);
- if (error == 0 && (flags & SOCK_CLOEXEC))
- fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
- fdpunlock(fdp);
- if (error != 0) {
- /*
- * Probably ran out of file descriptors. Wakeup
- * so some other process might have a chance at it.
- */
- wakeup_one(&head->so_timeo);
- goto bad;
- }
-
nam = m_get(M_WAIT, MT_SONAME);
/*
- * Check whether the queue emptied while we slept: falloc() or
- * m_get() may have blocked, allowing the connection to be reset
- * or another thread or process to accept it. If so, start over.
+ * Check whether the queue emptied while we slept: m_get() may have
+ * blocked, allowing the connection to be reset or another thread or
+ * process to accept it. If so, start over.
*/
if (head->so_qlen == 0) {
NET_UNLOCK(s);
m_freem(nam);
- fdplock(fdp);
- fdremove(fdp, tmpfd);
- closef(fp, p);
- fdpunlock(fdp);
+ nam = NULL;
goto redo;
}
@@ -360,24 +353,20 @@ redo:
error = soaccept(so, nam);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
-
+ if (!error) {
+ (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&nflag, p);
+ FILE_SET_MATURE(fp, p);
+ *retval = tmpfd;
+ }
+out:
+ NET_UNLOCK(s);
+ m_freem(nam);
if (error) {
- /* if an error occurred, free the file descriptor */
- NET_UNLOCK(s);
- m_freem(nam);
fdplock(fdp);
fdremove(fdp, tmpfd);
closef(fp, p);
fdpunlock(fdp);
- goto out;
}
- (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&nflag, p);
- FILE_SET_MATURE(fp, p);
- *retval = tmpfd;
- m_freem(nam);
-bad:
- NET_UNLOCK(s);
-out:
FRELE(headfp, p);
return (error);
}