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

Fix locking in soisconnected().

Browse Source 
When a newborn socket moves from incomplete queue to complete
one, we need to obtain the listening socket lock after the child,
which is a wrong order.  The old code did that in potentially
endless loop of mtx_trylock().  The new one does only one attempt
of mtx_trylock(), and in case of failure references listening
socket, unlocks child and locks everything in right order.  In
case if listening socket shuts down during that, just bail out.

Reported & tested by:	Jason Eggleston <jeggleston llnw.com>
Reported & tested by:	Jason Wolfe <jason llnw.com>
This commit is contained in:
glebius 2017-09-14 18:05:54 +00:00
parent 54f410cc0e
commit 13e2631492
1 changed files with 31 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
sys/kern/uipc_socket.c
View File

@ -3688,24 +3688,41 @@ soisconnecting(struct socket *so)
void
soisconnected(struct socket *so)
{
struct socket *head;
int ret;
/*
* XXXGL: this is the only place where we acquire socket locks
* in reverse order: first child, then listening socket. To
* avoid possible LOR, use try semantics.
*/
restart:
SOCK_LOCK(so);
if ((head = so->so_listen) != NULL &&
__predict_false(SOLISTEN_TRYLOCK(head) == 0)) {
SOCK_UNLOCK(so);
goto restart;
}
so->so_state &= ~(SS_ISCONNECTING|SS_ISDISCONNECTING|SS_ISCONFIRMING);
so->so_state |= SS_ISCONNECTED;
if (head != NULL && (so->so_qstate == SQ_INCOMP)) {
if (so->so_qstate == SQ_INCOMP) {
struct socket *head = so->so_listen;
int ret;
KASSERT(head, ("%s: so %p on incomp of NULL", __func__, so));
/*
* Promoting a socket from incomplete queue to complete, we
* need to go through reverse order of locking. We first do
* trylock, and if that doesn't succeed, we go the hard way
* leaving a reference and rechecking consistency after proper
* locking.
*/
if (__predict_false(SOLISTEN_TRYLOCK(head) == 0)) {
soref(head);
SOCK_UNLOCK(so);
SOLISTEN_LOCK(head);
SOCK_LOCK(so);
if (__predict_false(head != so->so_listen)) {
/*
* The socket went off the listen queue,
* should be lost race to close(2) of sol.
* The socket is about to soabort().
*/
SOCK_UNLOCK(so);
sorele(head);
return;
}
/* Not the last one, as so holds a ref. */
refcount_release(&head->so_count);
}
again:
if ((so->so_options & SO_ACCEPTFILTER) == 0) {
TAILQ_REMOVE(&head->sol_incomp, so, so_list);
@ -3734,8 +3751,6 @@ soisconnected(struct socket *so)
}
return;
}
if (head != NULL)
SOLISTEN_UNLOCK(head);
SOCK_UNLOCK(so);
wakeup(&so->so_timeo);
sorwakeup(so);