]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Don't use rwlocks for reference counting
authorOndřej Surý <ondrej@sury.org>
Mon, 20 May 2019 14:55:42 +0000 (16:55 +0200)
committerMark Andrews <marka@isc.org>
Thu, 10 Dec 2020 06:31:19 +0000 (06:31 +0000)
    WARNING: ThreadSanitizer: data race
    Write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1):
    #0 memset <null>
    #1 mem_put lib/isc/mem.c:819
    #2 isc___mem_free lib/isc/mem.c:1662
    #3 isc__mem_free lib/isc/mem.c:3078
    #4 isc___mem_putanddetach lib/isc/mem.c:1221
    #5 isc__mem_putanddetach lib/isc/mem.c:3033
    #6 destroyring lib/dns/tsig.c:494
    #7 dns_tsigkeyring_dumpanddetach lib/dns/tsig.c:665
    #8 destroy lib/dns/view.c:392
    #9 dns_view_weakdetach lib/dns/view.c:704
    #10 zone_free lib/dns/zone.c:1152
    #11 zone_shutdown lib/dns/zone.c:13123
    #12 dispatch lib/isc/task.c:1157
    #13 run lib/isc/task.c:1331
    #14 <null> <null>

    Previous atomic read of size 8 at 0x000000000001 by thread T2:
    #0 __tsan_atomic64_load <null>
    #1 isc_rwlock_unlock lib/isc/rwlock.c:612
    #2 dns_tsigkeyring_dumpanddetach lib/dns/tsig.c:632
    #3 destroy lib/dns/view.c:392
    #4 dns_view_weakdetach lib/dns/view.c:704
    #5 zone_free lib/dns/zone.c:1149
    #6 zone_shutdown lib/dns/zone.c:13123
    #7 dispatch lib/isc/task.c:1157
    #8 run lib/isc/task.c:1331
    #9 <null> <null>

lib/dns/include/dns/tsig.h
lib/dns/tsig.c

index 4c05be49e748b414d372b2334c79c4f8aff90851..2fa1ab1a978e5e8229d455640c4ae91728ed2e85 100644 (file)
@@ -69,7 +69,7 @@ struct dns_tsig_keyring {
        unsigned int generated;
        unsigned int maxgenerated;
        ISC_LIST(dns_tsigkey_t) lru;
-       unsigned int references;
+       isc_refcount_t references;
 };
 
 struct dns_tsigkey {
index 4d4b68f2fa3ddf91d8b36ad6c8b5e7aa07c16f71..540b03f8956adb8268e196ca5b6f6ca876c8f61c 100644 (file)
@@ -368,8 +368,9 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
        if (ring != NULL)
                refs++;
        ret = isc_refcount_init(&tkey->refs, refs);
-       if (ret != ISC_R_SUCCESS)
+       if (ret != ISC_R_SUCCESS) {
                goto cleanup_creator;
+       }
 
        tkey->generated = generated;
        tkey->inception = inception;
@@ -407,8 +408,9 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
 
  cleanup_refs:
        tkey->magic = 0;
-       while (refs-- > 0)
+       while (refs-- > 0) {
                isc_refcount_decrement(&tkey->refs, NULL);
+       }
        isc_refcount_destroy(&tkey->refs);
  cleanup_creator:
        if (tkey->key != NULL)
@@ -625,14 +627,10 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp) {
        ring = *ringp;
        *ringp = NULL;
 
-       RWLOCK(&ring->lock, isc_rwlocktype_write);
-       INSIST(ring->references > 0);
-       ring->references--;
-       references = ring->references;
-       RWUNLOCK(&ring->lock, isc_rwlocktype_write);
-
-       if (references != 0)
+       isc_refcount_decrement(&ring->references, &references);
+       if (references > 0) {
                return (DNS_R_CONTINUE);
+       }
 
        isc_stdtime_get(&now);
        dns_name_init(&foundname, NULL);
@@ -1932,7 +1930,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) {
        ring->maxgenerated = DNS_TSIG_MAXGENERATEDKEYS;
        ISC_LIST_INIT(ring->lru);
        isc_mem_attach(mctx, &ring->mctx);
-       ring->references = 1;
+       isc_refcount_init(&ring->references, 1);
 
        *ringp = ring;
        return (ISC_R_SUCCESS);
@@ -1945,8 +1943,9 @@ dns_tsigkeyring_add(dns_tsig_keyring_t *ring, dns_name_t *name,
        isc_result_t result;
 
        result = keyring_add(ring, name, tkey);
-       if (result == ISC_R_SUCCESS)
+       if (result == ISC_R_SUCCESS) {
                isc_refcount_increment(&tkey->refs, NULL);
+       }
 
        return (result);
 }
@@ -1957,12 +1956,9 @@ dns_tsigkeyring_attach(dns_tsig_keyring_t *source, dns_tsig_keyring_t **target)
        REQUIRE(source != NULL);
        REQUIRE(target != NULL && *target == NULL);
 
-       RWLOCK(&source->lock, isc_rwlocktype_write);
-       INSIST(source->references > 0);
-       source->references++;
-       INSIST(source->references > 0);
+       isc_refcount_increment(&source->references, NULL);
+
        *target = source;
-       RWUNLOCK(&source->lock, isc_rwlocktype_write);
 }
 
 void
@@ -1976,14 +1972,10 @@ dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp) {
        ring = *ringp;
        *ringp = NULL;
 
-       RWLOCK(&ring->lock, isc_rwlocktype_write);
-       INSIST(ring->references > 0);
-       ring->references--;
-       references = ring->references;
-       RWUNLOCK(&ring->lock, isc_rwlocktype_write);
-
-       if (references == 0)
+       isc_refcount_decrement(&ring->references, &references);
+       if (references == 0) {
                destroyring(ring);
+       }
 }
 
 void