From 34e14abe3a88863e31ac160b6d321722d89bfd33 Mon Sep 17 00:00:00 2001 From: quink-black Date: Tue, 11 May 2021 18:45:57 +0800 Subject: [PATCH] [core] Fix use-after-free in rendezvous queue (#1919) 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(..). --- srtcore/queue.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/srtcore/queue.cpp b/srtcore/queue.cpp index 61400df22..8fe5809bc 100644 --- a/srtcore/queue.cpp +++ b/srtcore/queue.cpp @@ -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); @@ -1100,7 +1095,13 @@ void CRendezvousQueue::updateConnStatus(EReadStatus rst, EConnectStatus cst, con for (vector::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