From 0450acc1b65442a0e904c895cf2875eacf409598 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 8 Sep 2020 12:11:06 +1000 Subject: [PATCH] Lock access to control->symtab to prevent data race WARNING: ThreadSanitizer: data race Read of size 8 at 0x000000000001 by thread T1: #0 isccc_symtab_foreach lib/isccc/symtab.c:277:14 #1 isccc_cc_cleansymtab lib/isccc/cc.c:954:2 #2 control_recvmessage bin/named/controlconf.c:477:2 #3 recv_data lib/isccc/ccmsg.c:110:2 #4 read_cb lib/isc/netmgr/tcp.c:769:4 #5 Previous write of size 8 at 0x000000000001 by thread T2: #0 isccc_symtab_define lib/isccc/symtab.c:242:2 #1 isccc_cc_checkdup lib/isccc/cc.c:1026:11 #2 control_recvmessage bin/named/controlconf.c:478:11 #3 recv_data lib/isccc/ccmsg.c:110:2 #4 read_cb lib/isc/netmgr/tcp.c:769:4 #5 Location is heap block of size 190352 at 0x000000000011 allocated by main thread: #0 malloc #1 isccc_symtab_create lib/isccc/symtab.c:76:18 #2 isccc_cc_createsymtab lib/isccc/cc.c:948:10 #3 named_controls_create bin/named/controlconf.c:1483:11 #4 named_server_create bin/named/server.c:10057:2 #5 setup bin/named/main.c:1256:2 #6 main bin/named/main.c:1523:2 Thread T1 (running) created by main thread at: #0 pthread_create #1 isc_thread_create lib/isc/pthreads/thread.c:73:8 #2 isc_nm_start lib/isc/netmgr/netmgr.c:215:3 #3 create_managers bin/named/main.c:909:15 #4 setup bin/named/main.c:1223:11 #5 main bin/named/main.c:1523:2 Thread T2 (running) created by main thread at: #0 pthread_create #1 isc_thread_create lib/isc/pthreads/thread.c:73:8 #2 isc_nm_start lib/isc/netmgr/netmgr.c:215:3 #3 create_managers bin/named/main.c:909:15 #4 setup bin/named/main.c:1223:11 #5 main bin/named/main.c:1523:2 SUMMARY: ThreadSanitizer: data race lib/isccc/symtab.c:277:14 in isccc_symtab_foreach --- bin/named/controlconf.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 706ec0b9ec..97b2c1efe7 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -109,6 +110,7 @@ struct named_controls { named_server_t *server; controllistenerlist_t listeners; bool shuttingdown; + isc_mutex_t symtab_lock; isccc_symtab_t *symtab; }; @@ -497,9 +499,11 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { /* * Duplicate suppression (required for UDP). */ + LOCK(&listener->controls->symtab_lock); isccc_cc_cleansymtab(listener->controls->symtab, conn->now); result = isccc_cc_checkdup(listener->controls->symtab, conn->request, conn->now); + UNLOCK(&listener->controls->symtab_lock); if (result != ISC_R_SUCCESS) { if (result == ISC_R_EXISTS) { result = ISCCC_R_DUPLICATE; @@ -1492,15 +1496,19 @@ named_controls_create(named_server_t *server, named_controls_t **ctrlsp) { isc_result_t result; named_controls_t *controls = isc_mem_get(mctx, sizeof(*controls)); - if (controls == NULL) { - return (ISC_R_NOMEMORY); - } - controls->server = server; + *controls = (named_controls_t){ + .server = server, + }; + ISC_LIST_INIT(controls->listeners); - controls->shuttingdown = false; - controls->symtab = NULL; + + isc_mutex_init(&controls->symtab_lock); + LOCK(&controls->symtab_lock); result = isccc_cc_createsymtab(&controls->symtab); + UNLOCK(&controls->symtab_lock); + if (result != ISC_R_SUCCESS) { + isc_mutex_destroy(&controls->symtab_lock); isc_mem_put(server->mctx, controls, sizeof(*controls)); return (result); } @@ -1515,6 +1523,9 @@ named_controls_destroy(named_controls_t **ctrlsp) { REQUIRE(ISC_LIST_EMPTY(controls->listeners)); + LOCK(&controls->symtab_lock); isccc_symtab_destroy(&controls->symtab); + UNLOCK(&controls->symtab_lock); + isc_mutex_destroy(&controls->symtab_lock); isc_mem_put(controls->server->mctx, controls, sizeof(*controls)); }