Skip to content

Commit

Permalink
[core] Fix use-after-free in rendezvous queue (#1919)
Browse files Browse the repository at this point in the history
A socket changing its m_bConnecting state from true to false will no longer be removed from the queue when closing. See CUDT::closeInternal().
Therefore it must be removed from the queue in CRendezvousQueue::updateConnStatus(..).
  • Loading branch information
quink-black committed May 11, 2021
1 parent de68ae9 commit 34e14ab
Showing 1 changed file with 6 additions and 5 deletions.
11 changes: 6 additions & 5 deletions srtcore/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1009,11 +1009,6 @@ void CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst, con
LinkStatusInfo fi = {i->m_pUDT, i->m_iID, ccerror, i->m_PeerAddr, -1};
ufailed.push_back(fi);

/*
* Setting m_bConnecting to false but keeping socket in rendezvous queue is not a good idea.
* Next CUDT::close will not remove it from rendezvous queue (because !m_bConnecting)
* and may crash here on next pass.
*/
// i_next was preincremented, but this is guaranteed to point to
// the element next to erased one.
i_next = m_lRendezvousID.erase(i);
Expand Down Expand Up @@ -1100,7 +1095,13 @@ void CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst, con
for (vector<LinkStatusInfo>::iterator i = ufailed.begin(); i != ufailed.end(); ++i)
{
HLOGC(cnlog.Debug, log << "updateConnStatus: COMPLETING dep objects update on failed @" << i->id);
/*
* Setting m_bConnecting to false but keeping socket in rendezvous queue is not a good idea.
* Next CUDT::close will not remove it from rendezvous queue (because !m_bConnecting)
* and may crash on next pass.
*/
i->u->m_bConnecting = false;
remove(i->u->m_SocketID);

// DO NOT close the socket here because in this case it might be
// unable to get status from at the right moment. Also only member
Expand Down

0 comments on commit 34e14ab

Please sign in to comment.