]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Lock access to control->symtab to prevent data race
authorMark Andrews <marka@isc.org>
Tue, 8 Sep 2020 02:11:06 +0000 (12:11 +1000)
committerMark Andrews <marka@isc.org>
Thu, 17 Sep 2020 09:31:41 +0000 (19:31 +1000)
    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

(cherry picked from commit 0450acc1b65442a0e904c895cf2875eacf409598)

bin/named/controlconf.c

index 94a89fdca7eca7528e61be9c474d064e4adad2d6..958b8027fa61058244d62f5193b1a734e64d7eac 100644 (file)
@@ -19,6 +19,7 @@
 #include <isc/event.h>
 #include <isc/file.h>
 #include <isc/mem.h>
+#include <isc/mutex.h>
 #include <isc/net.h>
 #include <isc/netaddr.h>
 #include <isc/nonce.h>
@@ -106,6 +107,7 @@ struct named_controls {
        named_server_t *server;
        controllistenerlist_t listeners;
        bool shuttingdown;
+       isc_mutex_t symtab_lock;
        isccc_symtab_t *symtab;
 };
 
@@ -440,8 +442,10 @@ control_recvmessage(isc_task_t *task, isc_event_t *event) {
        /*
         * Duplicate suppression (required for UDP).
         */
+       LOCK(&listener->controls->symtab_lock);
        isccc_cc_cleansymtab(listener->controls->symtab, now);
        result = isccc_cc_checkdup(listener->controls->symtab, request, now);
+       UNLOCK(&listener->controls->symtab_lock);
        if (result != ISC_R_SUCCESS) {
                if (result == ISC_R_EXISTS) {
                        result = ISCCC_R_DUPLICATE;
@@ -1522,15 +1526,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);
        }
@@ -1545,6 +1553,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));
 }