Skip to content

Commit bd29b80

Browse files
committed
Fix double deref in connection pool error path
1 parent 2e836c1 commit bd29b80

File tree

1 file changed

+49
-9
lines changed

1 file changed

+49
-9
lines changed

src/core/connection_pool.c

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,39 @@ QuicConnPoolGetInterfaceIndexForLocalAddress(
236236
return Status;
237237
}
238238

239+
//
240+
// Queue a close operation on the connection worker thread,
241+
// optionally waiting for its completion.
242+
//
243+
static
244+
_IRQL_requires_max_(PASSIVE_LEVEL)
245+
void
246+
QuicConnPoolQueueConnectionClose(
247+
_In_ QUIC_CONNECTION* Connection,
248+
_In_ const BOOLEAN WaitForCompletion
249+
)
250+
{
251+
Connection->CloseOper.Type = QUIC_OPER_TYPE_API_CALL;
252+
Connection->CloseOper.FreeAfterProcess = FALSE;
253+
Connection->CloseOper.API_CALL.Context = &Connection->CloseApiContext;
254+
Connection->CloseApiContext.Type = QUIC_API_TYPE_CONN_CLOSE;
255+
Connection->CloseApiContext.Status = NULL;
256+
257+
CXPLAT_EVENT CompletionEvent = {0};
258+
if (WaitForCompletion) {
259+
CxPlatEventInitialize(&CompletionEvent, TRUE, FALSE);
260+
Connection->CloseApiContext.Completed = &CompletionEvent;
261+
}
262+
263+
QuicConnQueueOper(Connection, &Connection->CloseOper);
264+
265+
if (WaitForCompletion) {
266+
CxPlatEventWaitForever(CompletionEvent);
267+
CxPlatEventUninitialize(CompletionEvent);
268+
}
269+
}
270+
271+
239272
static
240273
_IRQL_requires_max_(PASSIVE_LEVEL)
241274
QUIC_STATUS
@@ -359,11 +392,12 @@ QuicConnPoolTryCreateConnection(
359392

360393
if (QUIC_FAILED(Status) && *Connection != NULL) {
361394
//
362-
// This connection has never left MsQuic back to the application,
363-
// so don't send any notifications to the application on close.
395+
// This connection has never left MsQuic back to the application.
396+
// Mark it as internally owned so no notification is sent to the app,
397+
// the closing logic will handle the final deref.
364398
//
365399
(*Connection)->State.ExternalOwner = FALSE;
366-
MsQuicConnectionClose((HQUIC)*Connection);
400+
QuicConnPoolQueueConnectionClose(*Connection, FALSE);
367401
*Connection = NULL;
368402
}
369403

@@ -673,13 +707,19 @@ MsQuicConnectionPoolCreate(
673707
}
674708

675709
CleanUpConnections:
676-
if (QUIC_FAILED(Status) &&
677-
(Config->Flags & QUIC_CONNECTION_POOL_FLAG_CLOSE_ON_FAILURE) != 0) {
678-
for (uint32_t i = 0; i < CreatedConnections; i++) {
679-
MsQuicConnectionClose((HQUIC)Connections[i]);
680-
Connections[i] = NULL;
710+
if (QUIC_FAILED(Status) &&
711+
(Config->Flags & QUIC_CONNECTION_POOL_FLAG_CLOSE_ON_FAILURE) != 0) {
712+
713+
//
714+
// Close every connection that was created.
715+
// The application will receive the shutdown notification.
716+
//
717+
for (uint32_t i = 0; i < CreatedConnections; i++) {
718+
QuicConnPoolQueueConnectionClose(Connections[i], TRUE);
719+
QuicConnRelease(Connections[i], QUIC_CONN_REF_HANDLE_OWNER);
720+
Connections[i] = NULL;
721+
}
681722
}
682-
}
683723

684724
Error:
685725
if (ServerNameCopy != NULL) {

0 commit comments

Comments
 (0)