Address data race over query_recurse.last

WARNING: ThreadSanitizer: data race
  Read of size 4 at 0x000000000001 by thread T1:
    #0 query_recurse bin/named/query.c:4291:15
    #1 query_find bin/named/query.c
    #2 ns_query_start bin/named/query.c:9675:8
    #3 client_request bin/named/client.c:3133:3
    #4 dispatch lib/isc/task.c:1157:7
    #5 run lib/isc/task.c:1331:2

  Previous write of size 4 at 0x000000000001 by thread T2:
    #0 query_recurse bin/named/query.c:4292:10
    #1 query_find bin/named/query.c
    #2 ns_query_start bin/named/query.c:9675:8
    #3 client_request bin/named/client.c:3133:3
    #4 dispatch lib/isc/task.c:1157:7
    #5 run lib/isc/task.c:1331:2

  Location is global 'query_recurse.last' of size 4 at 0x000000000001

diff --git a/bin/named/query.c b/bin/named/query.c
index 203f1e611a73e180f5addb79c860f4f6cbb9f93d..aaed6194e5cba68f91e4e9c5b086582a2d52f405 100644 (file)
--- a/bin/named/query.c
+++ b/bin/named/query.c
@@ -18,6 +18,7 @@
 
 #include <isc/hex.h>
 #include <isc/mem.h>
+#include <isc/platform.h>
 #include <isc/print.h>
 #include <isc/rwlock.h>
 #include <isc/serial.h>
@@ -61,6 +62,29 @@
 #include <named/sortlist.h>
 #include <named/xfrout.h>
 
+#if defined(ISC_PLATFORM_HAVESTDATOMIC)
+#if defined(__cplusplus)
+#include <isc/stdatomic.h>
+#else
+#include <stdatomic.h>
+#endif
+#define last_load(x) atomic_load((x))
+#define        last_cmpxchg(x, e, r) atomic_compare_exchange_strong((x), (e), r)
+#elif defined(ISC_PLATFORM_HAVEXADD) && defined(ISC_PLATFORM_HAVECMPXCHG)
+#define last_load(x) (isc_stdtime_t)isc_atomic_xadd((int32_t*)(x), 0)
+#define last_cmpxchg(x, e, r) isc_atomic_cmpxchg((int32_t*)x, (*(int32_t*)(e)), (int32_t)(r))
+#else
+#define last_load(x) (*(x))
+static inline bool
+last_cmpxchg(isc_stdtime_t *x, isc_stdtime_t *e, isc_stdtime_t r) {
+       if (*x == *e) {
+               *x = r;
+               return (true);
+       }
+       return (false);
+}
+#endif
+
 #if 0
 /*
  * It has been recommended that DNS64 be changed to return excluded
@@ -4253,6 +4277,33 @@ query_prefetch(ns_client_t *client, dns_name_t *qname,
        dns_rdataset_clearprefetch(rdataset);
 }
 
+
+#if defined(ISC_PLATFORM_HAVESTDATOMIC)
+static void
+log_quota(ns_client_t *client, _Atomic(isc_stdtime_t) *last, isc_stdtime_t now,
+         const char *fmt, const char *tail)
+#else
+static void
+log_quota(ns_client_t *client, isc_stdtime_t *last, isc_stdtime_t now,
+         const char *fmt, const char *tail)
+#endif
+{
+       isc_stdtime_t old = last_load(last);
+       if (now > old || (old + 1) > now) {
+               if (last_cmpxchg(last, &old, now)) {
+                       LOCK(&client->recursionquota->lock);
+                       ns_client_log(client, NS_LOGCATEGORY_CLIENT,
+                                             NS_LOGMODULE_QUERY,
+                                             ISC_LOG_WARNING, fmt,
+                                             client->recursionquota->used,
+                                             client->recursionquota->soft,
+                                             client->recursionquota->max,
+                                             tail);
+                       UNLOCK(&client->recursionquota->lock);
+               }
+       }
+}
+
 static isc_result_t
 query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
              dns_name_t *qdomain, dns_rdataset_t *nameservers,
@@ -4285,39 +4336,29 @@ query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
                }
 
                if  (result == ISC_R_SOFTQUOTA) {
+#ifdef ISC_PLATFORM_HAVESTDATOMIC
+                       static _Atomic(isc_stdtime_t) last = 0;
+#else
                        static isc_stdtime_t last = 0;
+#endif
                        isc_stdtime_t now;
                        isc_stdtime_get(&now);
-                       if (now != last) {
-                               last = now;
-                               ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-                                             NS_LOGMODULE_QUERY,
-                                             ISC_LOG_WARNING,
-                                             "recursive-clients soft limit "
-                                             "exceeded (%d/%d/%d), "
-                                             "aborting oldest query",
-                                             client->recursionquota->used,
-                                             client->recursionquota->soft,
-                                             client->recursionquota->max);
-                       }
+                       log_quota(client, &last, now,
+                                 "recursive-clients soft limit exceeded "
+                                 "(%d/%d/%d) %s", "aborting oldest query");
                        ns_client_killoldestquery(client);
                        result = ISC_R_SUCCESS;
                } else if (result == ISC_R_QUOTA) {
+#ifdef ISC_PLATFORM_HAVESTDATOMIC
+                       static _Atomic(isc_stdtime_t) last = 0;
+#else
                        static isc_stdtime_t last = 0;
+#endif
                        isc_stdtime_t now;
                        isc_stdtime_get(&now);
-                       if (now != last) {
-                               last = now;
-                               ns_client_log(client, NS_LOGCATEGORY_CLIENT,
-                                             NS_LOGMODULE_QUERY,
-                                             ISC_LOG_WARNING,
-                                             "no more recursive clients "
-                                             "(%d/%d/%d): %s",
-                                             ns_g_server->recursionquota.used,
-                                             ns_g_server->recursionquota.soft,
-                                             ns_g_server->recursionquota.max,
-                                             isc_result_totext(result));
-                       }
+                       log_quota(client, &last, now,
+                                 "no more recursive clients (%d/%d/%d): %s",
+                                 isc_result_totext(result));
                        ns_client_killoldestquery(client);
                }
                if (result == ISC_R_SUCCESS && !client->mortal &&