Commit 7e39ead1 authored by Mark Andrews's avatar Mark Andrews
Browse files

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 <null> <null>

  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 <null> <null>

  Location is heap block of size 190352 at 0x000000000011 allocated by main thread:
    #0 malloc <null>
    #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 <null>
    #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 <null>
    #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
parent b5abdee5
Pipeline #51744 failed with stages
in 16 minutes and 39 seconds
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
#include <isc/event.h> #include <isc/event.h>
#include <isc/file.h> #include <isc/file.h>
#include <isc/mem.h> #include <isc/mem.h>
#include <isc/mutex.h>
#include <isc/net.h> #include <isc/net.h>
#include <isc/netaddr.h> #include <isc/netaddr.h>
#include <isc/netmgr.h> #include <isc/netmgr.h>
...@@ -109,6 +110,7 @@ struct named_controls { ...@@ -109,6 +110,7 @@ struct named_controls {
named_server_t *server; named_server_t *server;
controllistenerlist_t listeners; controllistenerlist_t listeners;
bool shuttingdown; bool shuttingdown;
isc_mutex_t symtab_lock;
isccc_symtab_t *symtab; isccc_symtab_t *symtab;
}; };
...@@ -495,9 +497,11 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { ...@@ -495,9 +497,11 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
/* /*
* Duplicate suppression (required for UDP). * Duplicate suppression (required for UDP).
*/ */
LOCK(&listener->controls->symtab_lock);
isccc_cc_cleansymtab(listener->controls->symtab, conn->now); isccc_cc_cleansymtab(listener->controls->symtab, conn->now);
result = isccc_cc_checkdup(listener->controls->symtab, conn->request, result = isccc_cc_checkdup(listener->controls->symtab, conn->request,
conn->now); conn->now);
UNLOCK(&listener->controls->symtab_lock);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
if (result == ISC_R_EXISTS) { if (result == ISC_R_EXISTS) {
result = ISCCC_R_DUPLICATE; result = ISCCC_R_DUPLICATE;
...@@ -1496,9 +1500,13 @@ named_controls_create(named_server_t *server, named_controls_t **ctrlsp) { ...@@ -1496,9 +1500,13 @@ named_controls_create(named_server_t *server, named_controls_t **ctrlsp) {
controls->server = server; controls->server = server;
ISC_LIST_INIT(controls->listeners); ISC_LIST_INIT(controls->listeners);
controls->shuttingdown = false; controls->shuttingdown = false;
isc_mutex_init(&controls->symtab_lock);
LOCK(&controls->symtab_lock);
controls->symtab = NULL; controls->symtab = NULL;
result = isccc_cc_createsymtab(&controls->symtab); result = isccc_cc_createsymtab(&controls->symtab);
UNLOCK(&controls->symtab_lock);
if (result != ISC_R_SUCCESS) { if (result != ISC_R_SUCCESS) {
isc_mutex_destroy(&controls->symtab_lock);
isc_mem_put(server->mctx, controls, sizeof(*controls)); isc_mem_put(server->mctx, controls, sizeof(*controls));
return (result); return (result);
} }
...@@ -1513,6 +1521,9 @@ named_controls_destroy(named_controls_t **ctrlsp) { ...@@ -1513,6 +1521,9 @@ named_controls_destroy(named_controls_t **ctrlsp) {
REQUIRE(ISC_LIST_EMPTY(controls->listeners)); REQUIRE(ISC_LIST_EMPTY(controls->listeners));
LOCK(&controls->symtab_lock);
isccc_symtab_destroy(&controls->symtab); isccc_symtab_destroy(&controls->symtab);
UNLOCK(&controls->symtab_lock);
isc_mutex_destroy(&controls->symtab_lock);
isc_mem_put(controls->server->mctx, controls, sizeof(*controls)); isc_mem_put(controls->server->mctx, controls, sizeof(*controls));
} }
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment