Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit d025b22

Browse files
nhormant8m
authored andcommitted
add locking around fake_now
fake_now in the quictestlib is read/written by potentially many threads, and as such should have a surrounding lock to prevent WAR/RAW errors as caught by tsan: 2023-11-03T16:27:23.7184999Z ================== 2023-11-03T16:27:23.7185290Z WARNING: ThreadSanitizer: data race (pid=18754) 2023-11-03T16:27:23.7185720Z Read of size 8 at 0x558f6f9fe970 by main thread: 2023-11-03T16:27:23.7186726Z #0 qtest_create_quic_connection_ex <null> (quicapitest+0x14aead) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7187665Z #1 qtest_create_quic_connection <null> (quicapitest+0x14b220) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7188567Z #2 test_quic_write_read quicapitest.c (quicapitest+0x150ee2) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7189561Z #3 run_tests <null> (quicapitest+0x2237ab) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7190294Z #4 main <null> (quicapitest+0x223d2b) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7190720Z 2023-11-03T16:27:23.7190902Z Previous write of size 8 at 0x558f6f9fe970 by thread T1: 2023-11-03T16:27:23.7191607Z #0 qtest_create_quic_connection_ex <null> (quicapitest+0x14aecf) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7192505Z #1 run_server_thread quictestlib.c (quicapitest+0x14b1d6) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7193361Z #2 thread_run quictestlib.c (quicapitest+0x14cadf) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7193848Z 2023-11-03T16:27:23.7194220Z Location is global 'fake_now.0' of size 8 at 0x558f6f9fe970 (quicapitest+0x1af4970) 2023-11-03T16:27:23.7194636Z 2023-11-03T16:27:23.7194816Z Thread T1 (tid=18760, running) created by main thread at: 2023-11-03T16:27:23.7195465Z #0 pthread_create <null> (quicapitest+0xca12d) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7196317Z #1 qtest_create_quic_connection_ex <null> (quicapitest+0x14adcb) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7197214Z #2 qtest_create_quic_connection <null> (quicapitest+0x14b220) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7198111Z #3 test_quic_write_read quicapitest.c (quicapitest+0x150ee2) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7198940Z #4 run_tests <null> (quicapitest+0x2237ab) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7199661Z #5 main <null> (quicapitest+0x223d2b) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) 2023-11-03T16:27:23.7200083Z 2023-11-03T16:27:23.7200862Z SUMMARY: ThreadSanitizer: data race (/home/runner/work/openssl/openssl/test/quicapitest+0x14aead) (BuildId: d06f7b04830b55de9c8482b398a1781472d1c7d5) in qtest_create_quic_connection_ex Reviewed-by: Hugo Landau <[email protected]> Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#22616) (cherry picked from commit 11179b3)
1 parent b4cf49c commit d025b22

File tree

1 file changed

+39
-6
lines changed

1 file changed

+39
-6
lines changed

test/helpers/quictestlib.c

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,16 @@ struct qtest_fault {
7474

7575
static void packet_plain_finish(void *arg);
7676
static void handshake_finish(void *arg);
77+
static OSSL_TIME qtest_get_time(void);
78+
static void qtest_reset_time(void);
7779

7880
static int using_fake_time = 0;
7981
static OSSL_TIME fake_now;
82+
static CRYPTO_RWLOCK *fake_now_lock = NULL;
8083

8184
static OSSL_TIME fake_now_cb(void *arg)
8285
{
83-
return fake_now;
86+
return qtest_get_time();
8487
}
8588

8689
static void noise_msg_callback(int write_p, int version, int content_type,
@@ -282,11 +285,14 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
282285
if (serverctx != NULL && !TEST_true(SSL_CTX_up_ref(serverctx)))
283286
goto err;
284287
tserver_args.ctx = serverctx;
288+
if (fake_now_lock == NULL) {
289+
fake_now_lock = CRYPTO_THREAD_lock_new();
290+
if (fake_now_lock == NULL)
291+
goto err;
292+
}
285293
if ((flags & QTEST_FLAG_FAKE_TIME) != 0) {
286294
using_fake_time = 1;
287-
fake_now = ossl_time_zero();
288-
/* zero time can have a special meaning, bump it */
289-
qtest_add_time(1);
295+
qtest_reset_time();
290296
tserver_args.now_cb = fake_now_cb;
291297
(void)ossl_quic_conn_set_override_now_cb(*cssl, fake_now_cb, NULL);
292298
} else {
@@ -331,7 +337,31 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
331337

332338
void qtest_add_time(uint64_t millis)
333339
{
340+
if (!CRYPTO_THREAD_write_lock(fake_now_lock))
341+
return;
334342
fake_now = ossl_time_add(fake_now, ossl_ms2time(millis));
343+
CRYPTO_THREAD_unlock(fake_now_lock);
344+
}
345+
346+
static OSSL_TIME qtest_get_time(void)
347+
{
348+
OSSL_TIME ret;
349+
350+
if (!CRYPTO_THREAD_read_lock(fake_now_lock))
351+
return ossl_time_zero();
352+
ret = fake_now;
353+
CRYPTO_THREAD_unlock(fake_now_lock);
354+
return ret;
355+
}
356+
357+
static void qtest_reset_time(void)
358+
{
359+
if (!CRYPTO_THREAD_write_lock(fake_now_lock))
360+
return;
361+
fake_now = ossl_time_zero();
362+
CRYPTO_THREAD_unlock(fake_now_lock);
363+
/* zero time can have a special meaning, bump it */
364+
qtest_add_time(1);
335365
}
336366

337367
QTEST_FAULT *qtest_create_injector(QUIC_TSERVER *ts)
@@ -399,17 +429,20 @@ int qtest_wait_for_timeout(SSL *s, QUIC_TSERVER *qtserv)
399429
*/
400430
if (!SSL_get_event_timeout(s, &tv, &cinf))
401431
return 0;
432+
402433
if (using_fake_time)
403-
now = fake_now;
434+
now = qtest_get_time();
404435
else
405436
now = ossl_time_now();
437+
406438
ctimeout = cinf ? ossl_time_infinite() : ossl_time_from_timeval(tv);
407439
stimeout = ossl_time_subtract(ossl_quic_tserver_get_deadline(qtserv), now);
408440
mintimeout = ossl_time_min(ctimeout, stimeout);
409441
if (ossl_time_is_infinite(mintimeout))
410442
return 0;
443+
411444
if (using_fake_time)
412-
fake_now = ossl_time_add(now, mintimeout);
445+
qtest_add_time(ossl_time2ms(mintimeout));
413446
else
414447
OSSL_sleep(ossl_time2ms(mintimeout));
415448

0 commit comments

Comments
 (0)