From 840cf7adb3ccc9f95fa0b36c1b3af3800af520c1 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 1 Oct 2020 10:39:49 +1000 Subject: [PATCH] Add missing rwlock calls when access keynode.initial and keynode.managed WARNING: ThreadSanitizer: data race Write of size 1 at 0x000000000001 by thread T1 (mutexes: write M1): #0 dns_keynode_trust lib/dns/keytable.c:836 #1 keyfetch_done lib/dns/zone.c:10187 #2 dispatch lib/isc/task.c:1152 #3 run lib/isc/task.c:1344 #4 Previous read of size 1 at 0x000000000001 by thread T2 (mutexes: read M2): #0 keynode_dslist_totext lib/dns/keytable.c:682 #1 dns_keytable_totext lib/dns/keytable.c:732 #2 named_server_dumpsecroots bin/named/server.c:11357 #3 named_control_docommand bin/named/control.c:264 #4 control_command bin/named/controlconf.c:390 #5 dispatch lib/isc/task.c:1152 #6 run lib/isc/task.c:1344 #7 Location is heap block of size 241 at 0x000000000010 allocated by thread T3: #0 malloc #1 default_memalloc lib/isc/mem.c:713 #2 mem_get lib/isc/mem.c:622 #3 mem_allocateunlocked lib/isc/mem.c:1268 #4 isc___mem_allocate lib/isc/mem.c:1288 #5 isc__mem_allocate lib/isc/mem.c:2453 #6 isc___mem_get lib/isc/mem.c:1037 #7 isc__mem_get lib/isc/mem.c:2432 #8 new_keynode lib/dns/keytable.c:346 #9 insert lib/dns/keytable.c:393 #10 dns_keytable_add lib/dns/keytable.c:421 #11 process_key bin/named/server.c:955 #12 load_view_keys bin/named/server.c:983 #13 configure_view_dnsseckeys bin/named/server.c:1140 #14 configure_view bin/named/server.c:5371 #15 load_configuration bin/named/server.c:9110 #16 loadconfig bin/named/server.c:10310 #17 named_server_reconfigcommand bin/named/server.c:10693 #18 named_control_docommand bin/named/control.c:250 #19 control_command bin/named/controlconf.c:390 #20 dispatch lib/isc/task.c:1152 #21 run lib/isc/task.c:1344 #22 Mutex M1 is already destroyed. Mutex M2 is already destroyed. Thread T1 (running) created by main thread at: #0 pthread_create #1 isc_thread_create pthreads/thread.c:73 #2 isc_taskmgr_create lib/isc/task.c:1434 #3 create_managers bin/named/main.c:915 #4 setup bin/named/main.c:1223 #5 main bin/named/main.c:1523 Thread T2 (running) created by main thread at: #0 pthread_create #1 isc_thread_create pthreads/thread.c:73 #2 isc_taskmgr_create lib/isc/task.c:1434 #3 create_managers bin/named/main.c:915 #4 setup bin/named/main.c:1223 #5 main bin/named/main.c:1523 Thread T3 (running) created by main thread at: #0 pthread_create #1 isc_thread_create pthreads/thread.c:73 #2 isc_taskmgr_create lib/isc/task.c:1434 #3 create_managers bin/named/main.c:915 #4 setup bin/named/main.c:1223 #5 main bin/named/main.c:1523 SUMMARY: ThreadSanitizer: data race lib/dns/keytable.c:836 in dns_keynode_trust --- lib/dns/keytable.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c index bd83ed8f19..3514026377 100644 --- a/lib/dns/keytable.c +++ b/lib/dns/keytable.c @@ -677,10 +677,12 @@ keynode_dslist_totext(dns_name_t *name, dns_keynode_t *keynode, dns_secalg_format(ds.algorithm, algbuf, sizeof(algbuf)); + RWLOCK(&keynode->rwlock, isc_rwlocktype_read); snprintf(obuf, sizeof(obuf), "%s/%s/%d ; %s%s\n", namebuf, algbuf, ds.key_tag, keynode->initial ? "initializing " : "", keynode->managed ? "managed" : "static"); + RWUNLOCK(&keynode->rwlock, isc_rwlocktype_read); result = putstr(text, obuf); if (result != ISC_R_SUCCESS) { @@ -802,38 +804,56 @@ cleanup: bool dns_keynode_dsset(dns_keynode_t *keynode, dns_rdataset_t *rdataset) { + bool result; REQUIRE(VALID_KEYNODE(keynode)); REQUIRE(rdataset == NULL || DNS_RDATASET_VALID(rdataset)); + RWLOCK(&keynode->rwlock, isc_rwlocktype_read); if (keynode->dslist != NULL) { if (rdataset != NULL) { keynode_clone(&keynode->dsset, rdataset); } - return (true); + result = true; + } else { + result = false; } - - return (false); + RWUNLOCK(&keynode->rwlock, isc_rwlocktype_read); + return (result); } bool dns_keynode_managed(dns_keynode_t *keynode) { + bool managed; + REQUIRE(VALID_KEYNODE(keynode)); - return (keynode->managed); + RWLOCK(&keynode->rwlock, isc_rwlocktype_read); + managed = keynode->managed; + RWUNLOCK(&keynode->rwlock, isc_rwlocktype_read); + + return (managed); } bool dns_keynode_initial(dns_keynode_t *keynode) { + bool initial; + REQUIRE(VALID_KEYNODE(keynode)); - return (keynode->initial); + RWLOCK(&keynode->rwlock, isc_rwlocktype_read); + initial = keynode->initial; + RWUNLOCK(&keynode->rwlock, isc_rwlocktype_read); + + return (initial); } void dns_keynode_trust(dns_keynode_t *keynode) { REQUIRE(VALID_KEYNODE(keynode)); + RWLOCK(&keynode->rwlock, isc_rwlocktype_write); keynode->initial = false; + RWUNLOCK(&keynode->rwlock, isc_rwlocktype_write); } static void