Hold qid->lock when calling deref_portentry() as

socket_search() need portentry to be unchanging.

    WARNING: ThreadSanitizer: data race
    Write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1):
    #0 deref_portentry lib/dns/dispatch.c:630
    #1 deactivate_dispsocket lib/dns/dispatch.c:861
    #2 udp_recv lib/dns/dispatch.c:1105
    #3 udp_exrecv lib/dns/dispatch.c:1028
    #4 dispatch lib/isc/task.c:1152
    #5 run lib/isc/task.c:1344
    #6 <null> <null>

    Previous read of size 8 at 0x000000000001 by thread T2 (mutexes: write M1, write M2):
    #0 socket_search lib/dns/dispatch.c:661
    #1 get_dispsocket lib/dns/dispatch.c:744
    #2 dns_dispatch_addresponse lib/dns/dispatch.c:3120
    #3 resquery_send lib/dns/resolver.c:2467
    #4 fctx_query lib/dns/resolver.c:2217
    #5 fctx_try lib/dns/resolver.c:4245
    #6 fctx_timeout lib/dns/resolver.c:4570
    #7 dispatch lib/isc/task.c:1152
    #8 run lib/isc/task.c:1344
    #9 <null> <null>

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index c4265aa5c9282e8537aacb9a89982889b6856a78..82c019168d07dcbf5f78deb22dbaa5a3754f170f 100644 (file)
--- a/lib/dns/dispatch.c
+++ b/lib/dns/dispatch.c
@@ -619,11 +619,10 @@ new_portentry(dns_dispatch_t *disp, in_port_t port) {
 }
 
 /*%
- * The caller must not hold the qid->lock.
+ * The caller must hold the qid->lock.
  */
 static void
 deref_portentry(dns_dispatch_t *disp, dispportentry_t **portentryp) {
-       dns_qid_t *qid;
        dispportentry_t *portentry = *portentryp;
        *portentryp = NULL;
 
@@ -631,13 +630,10 @@ deref_portentry(dns_dispatch_t *disp, dispportentry_t **portentryp) {
        REQUIRE(portentry != NULL);
 
        if (isc_refcount_decrement(&portentry->refs) == 1) {
-               qid = DNS_QID(disp);
-               LOCK(&qid->lock);
                ISC_LIST_UNLINK(disp->port_table[portentry->port %
                                                 DNS_DISPATCH_PORTTABLESIZE],
                                portentry, link);
                isc_mempool_put(disp->portpool, portentry);
-               UNLOCK(&qid->lock);
        }
 }
 
@@ -777,9 +773,9 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest,
        if (result == ISC_R_SUCCESS) {
                dispsock->socket = sock;
                dispsock->host = *dest;
-               dispsock->portentry = portentry;
                dispsock->bucket = bucket;
                LOCK(&qid->lock);
+               dispsock->portentry = portentry;
                ISC_LIST_APPEND(qid->sock_table[bucket], dispsock, blink);
                UNLOCK(&qid->lock);
                *dispsockp = dispsock;
@@ -805,7 +801,7 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest,
 static void
 destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) {
        dispsocket_t *dispsock;
-       dns_qid_t *qid;
+       dns_qid_t *qid = DNS_QID(disp);
 
        /*
         * The dispatch must be locked.
@@ -819,13 +815,15 @@ destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) {
        disp->nsockets--;
        dispsock->magic = 0;
        if (dispsock->portentry != NULL) {
+               /* socket_search() tests and dereferences portentry. */
+               LOCK(&qid->lock);
                deref_portentry(disp, &dispsock->portentry);
+               UNLOCK(&qid->lock);
        }
        if (dispsock->socket != NULL) {
                isc_socket_detach(&dispsock->socket);
        }
        if (ISC_LINK_LINKED(dispsock, blink)) {
-               qid = DNS_QID(disp);
                LOCK(&qid->lock);
                ISC_LIST_UNLINK(qid->sock_table[dispsock->bucket], dispsock,
                                blink);
@@ -844,7 +842,7 @@ destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) {
 static void
 deactivate_dispsocket(dns_dispatch_t *disp, dispsocket_t *dispsock) {
        isc_result_t result;
-       dns_qid_t *qid;
+       dns_qid_t *qid = DNS_QID(disp);
 
        /*
         * The dispatch must be locked.
@@ -856,14 +854,16 @@ deactivate_dispsocket(dns_dispatch_t *disp, dispsocket_t *dispsock) {
        }
 
        INSIST(dispsock->portentry != NULL);
+       /* socket_search() tests and dereferences portentry. */
+       LOCK(&qid->lock);
        deref_portentry(disp, &dispsock->portentry);
+       UNLOCK(&qid->lock);
 
        if (disp->nsockets > DNS_DISPATCH_POOLSOCKS) {
                destroy_dispsocket(disp, &dispsock);
        } else {
                result = isc_socket_close(dispsock->socket);
 
-               qid = DNS_QID(disp);
                LOCK(&qid->lock);
                ISC_LIST_UNLINK(qid->sock_table[dispsock->bucket], dispsock,
                                blink);