On 20/01/17(Fri) 22:25, Alexander Bluhm wrote:
> On Thu, Jan 19, 2017 at 07:14:59PM +1000, Martin Pieuchot wrote:
> > Index: kern/uipc_syscalls.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> > retrieving revision 1.144
> > diff -u -p -r1.144 uipc_syscalls.c
> > --- kern/uipc_syscalls.c 29 Dec 2016 12:12:43 -0000 1.144
> > +++ kern/uipc_syscalls.c 19 Jan 2017 09:03:56 -0000
> > @@ -278,6 +278,7 @@ doaccept(struct proc *p, int sock, struc
> >
> > headfp = fp;
> > redo:
> > + fdplock(fdp);
> > NET_LOCK(s);
> > head = headfp->f_data;
> > if (isdnssocket(head) || (head->so_options & SO_ACCEPTCONN) == 0) {
> ...
>
> There we have
> error = rwsleep(&head->so_timeo, &netlock, PSOCK | PCATCH,
> "netcon", 0);
>
> You release the netlock during sleep but still hold the fdplock.
> This leads to a deadlock of userland during
> /usr/src/regress/lib/libpthread/socket/1.
>
> 277134 socket1 native 0xffff800003accfd8 thrsleep
> 236121 socket1 native 0xffffff00023eab56 netcon
> 220469 socket1 native 0xffffff0004d53a98 fdlock
>
> ddb{0}> trace /p 0t236121
> sleep_finish() at sleep_finish+0xb1
> rwsleep() at rwsleep+0x8e
> doaccept() at doaccept+0x37b
> syscall() at syscall+0x27b
> --- syscall (number 30) ---
>
> ddb{0}> trace /p 0t220469
> sleep_finish() at sleep_finish+0xb1
> rw_enter() at rw_enter+0x1cb
> sys_socket() at sys_socket+0xbf
> syscall() at syscall+0x27b
> --- syscall (number 97) ---
Here's another way to fix the problem, call falloc() before grabbing the
NET_LOCK().
Comments?
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.144
diff -u -p -r1.144 uipc_syscalls.c
--- kern/uipc_syscalls.c 29 Dec 2016 12:12:43 -0000 1.144
+++ kern/uipc_syscalls.c 21 Jan 2017 00:38:18 -0000
@@ -265,7 +265,7 @@ doaccept(struct proc *p, int sock, struc
{
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,35 @@ doaccept(struct proc *p, int sock, struc
return (error);
headfp = fp;
+ head = headfp->f_data;
+
+ 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);
+ 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,34 +314,19 @@ 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);
/*
@@ -360,24 +361,20 @@ redo:
error = soaccept(so, nam);
if (!error && name != NULL)
error = copyaddrout(p, nam, name, namelen, anamelen);
-
- if (error) {
- /* if an error occurred, free the file descriptor */
- NET_UNLOCK(s);
- m_freem(nam);
+ 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 != 0) {
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);
}
