From bea1326cdc10a1cac22acd788395adf6f2a5584c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 1 Oct 2020 15:11:32 +1000 Subject: [PATCH] Lock access to listener->connections as it is accessed from multiple threads with libuv. WARNING: ThreadSanitizer: data race Write of size 8 at 0x000000000001 by thread T1: #0 conn_reset bin/named/controlconf.c:574 #1 isc_nmhandle_detach netmgr/netmgr.c:1257 #2 isc__nm_uvreq_put netmgr/netmgr.c:1389 #3 tcp_send_cb netmgr/tcp.c:1030 #4 #5 Previous read of size 8 at 0x000000000001 by thread T2: #0 conn_reset bin/named/controlconf.c:574 #1 isc_nmhandle_detach netmgr/netmgr.c:1257 #2 control_recvmessage bin/named/controlconf.c:556 #3 recv_data lib/isccc/ccmsg.c:110 #4 isc__nm_tcp_shutdown netmgr/tcp.c:1161 #5 shutdown_walk_cb netmgr/netmgr.c:1511 #6 uv_walk #7 process_queue netmgr/netmgr.c:656 #8 process_normal_queue netmgr/netmgr.c:582 #9 process_queues netmgr/netmgr.c:590 #10 async_cb netmgr/netmgr.c:548 #11 #12 Location is heap block of size 265 at 0x000000000017 allocated by thread T3: #0 malloc #1 default_memalloc lib/isc/mem.c:713 #2 mem_get lib/isc/mem.c:622 #3 isc___mem_get lib/isc/mem.c:1044 #4 isc__mem_get lib/isc/mem.c:2432 #5 add_listener bin/named/controlconf.c:1127 #6 named_controls_configure bin/named/controlconf.c:1324 #7 load_configuration bin/named/server.c:9181 #8 run_server bin/named/server.c:9819 #9 dispatch lib/isc/task.c:1152 #10 run lib/isc/task.c:1344 #11 Thread T1 (running) created by main thread at: #0 pthread_create #1 isc_thread_create pthreads/thread.c:73 #2 isc_nm_start netmgr/netmgr.c:232 #3 create_managers bin/named/main.c:909 #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_nm_start netmgr/netmgr.c:232 #3 create_managers bin/named/main.c:909 #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 bin/named/controlconf.c:574 in conn_reset --- bin/named/controlconf.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 0b38b51a7f..5a14b100bd 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -96,6 +96,7 @@ struct controllistener { dns_acl_t *acl; bool exiting; controlkeylist_t keys; + isc_mutex_t connections_lock; controlconnectionlist_t connections; isc_socktype_t type; uint32_t perm; @@ -154,14 +155,19 @@ free_listener(controllistener_t *listener) { if (listener->acl != NULL) { dns_acl_detach(&listener->acl); } + isc_mutex_destroy(&listener->connections_lock); isc_mem_putanddetach(&listener->mctx, listener, sizeof(*listener)); } static void maybe_free_listener(controllistener_t *listener) { + LOCK(&listener->connections_lock); if (listener->exiting && ISC_LIST_EMPTY(listener->connections)) { + UNLOCK(&listener->connections_lock); free_listener(listener); + } else { + UNLOCK(&listener->connections_lock); } } @@ -571,7 +577,9 @@ conn_reset(void *arg) { return; } + LOCK(&listener->connections_lock); ISC_LIST_UNLINK(listener->connections, conn, link); + UNLOCK(&listener->connections_lock); #ifdef ENABLE_AFL if (named_g_fuzz_type == isc_fuzz_rndc) { named_fuzz_notify(); @@ -629,7 +637,9 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { goto cleanup; } + LOCK(&listener->connections_lock); ISC_LIST_APPEND(listener->connections, conn, link); + UNLOCK(&listener->connections_lock); return (ISC_R_SUCCESS); cleanup: @@ -1129,6 +1139,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, .address = *addr, .type = type }; isc_mem_attach(mctx, &listener->mctx); + isc_mutex_init(&listener->connections_lock); ISC_LINK_INIT(listener, link); ISC_LIST_INIT(listener->keys); ISC_LIST_INIT(listener->connections);