Fix locking in soisconnected().
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:
parent
bcf2b954c3
commit
584ab65a75
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=323594
@ -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);
|
||||
|
Loading…
Reference in New Issue
Block a user