Skip to content

Commit ece1f10

Browse files
committed
Spin in DataTest waiting for connections to be established
As the added comment indicates, The QuicTestConnectAndPing function locally allocates Secrets for the client and server connection for a given 0RTT test. However, it doesn't wait for the connection to be fully established before deciding the test is successful and returning. Because the allocation of those Secrets was done with a unique_ptr, leaving the scope of the function automatically deletes that memory, which another thread may still be referencing while processing data in the TLS backend, leading to ASAN errors. Fix it by ensuring that the connection is fully established prior to returinging (which implies that the TLS backend has completed filling out secrets, and dropped any reference to the TlsSecrets pointer
1 parent 7c82ab4 commit ece1f10

1 file changed

Lines changed: 21 additions & 0 deletions

File tree

src/test/lib/DataTest.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,27 @@ QuicTestConnectAndPing(
626626
ServerSecret->ClientEarlyTrafficSecret,
627627
ClientSecret->SecretLength));
628628
Match = true;
629+
//
630+
// This test dynamically allocates ClientSecrets and ServerSecrets
631+
// as a unique_ptr, which then gets passed and used by the underlying TLS
632+
// backend. However, this code doesn't wait for the connections to finish
633+
// getting established before exiting. As such it may occur that another
634+
// thread continues establishing the connection while this function exits,
635+
// which automatically deletes the allcoated memory in the Secrets, leading
636+
// to a potential NULL pointer deref in the TLS backend. We could shutdown
637+
// the connections here, but given that they're not yet fully established,
638+
// that on occaision fails. Instead, just spin here for a moment waiting
639+
// for the Client and Server Traffic Secrets to get set, at which point
640+
// we are guaranteed that the TLS backend will drop any reference to the
641+
// TLS pointer, so its safe to delete.
642+
//
643+
while (ClientSecret->IsSet.ClientTrafficSecret0 == 0 ||
644+
ClientSecret->IsSet.ServerTrafficSecret0 == 0 ||
645+
ServerSecret->IsSet.ClientTrafficSecret0 == 0 ||
646+
ServerSecret->IsSet.ServerTrafficSecret0 == 0) {
647+
CxPlatSleep(500); // Sleep for a little bit
648+
}
649+
629650
}
630651
}
631652
if (!Match) {

0 commit comments

Comments
 (0)