From: Ondřej Surý <ondrej@sury.org>
Date: Mon, 13 May 2019 17:36:02 +0000 (+0700)
Subject: bin/dnssec/dnssec-signzone.c: Protect global variables by making them atomic
X-Git-Tag: v9.15.2~11^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b104a9bc503033b4c3174f088e2c5bdbc5d8bb51;p=thirdparty%2Fbind9.git

bin/dnssec/dnssec-signzone.c: Protect global variables by making them atomic

Both global shuttingdown and finished bool variables were prone to data race
(as reported by ThreadSanitizer).  The commit makes them both atomic.
---

diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c
index f3a24cb1c33..e518ab3720b 100644
--- a/bin/dnssec/dnssec-signzone.c
+++ b/bin/dnssec/dnssec-signzone.c
@@ -32,6 +32,7 @@
 #include <unistd.h>
 
 #include <isc/app.h>
+#include <isc/atomic.h>
 #include <isc/base32.h>
 #include <isc/commandline.h>
 #include <isc/event.h>
@@ -45,8 +46,8 @@
 #include <isc/print.h>
 #include <isc/random.h>
 #include <isc/rwlock.h>
-#include <isc/serial.h>
 #include <isc/safe.h>
+#include <isc/serial.h>
 #include <isc/stdio.h>
 #include <isc/string.h>
 #include <isc/task.h>
@@ -155,7 +156,8 @@ static unsigned char *gsalt = saltbuf;
 static size_t salt_length = 0;
 static isc_task_t *master = NULL;
 static unsigned int ntasks = 0;
-static bool shuttingdown = false, finished = false;
+static atomic_bool shuttingdown = ATOMIC_VAR_INIT(false);
+static atomic_bool finished = ATOMIC_VAR_INIT(false);
 static bool nokeys = false;
 static bool removefile = false;
 static bool generateds = false;
@@ -1454,11 +1456,12 @@ signapex(void) {
 	cleannode(gdb, gversion, node);
 	dns_db_detachnode(gdb, &node);
 	result = dns_dbiterator_first(gdbiter);
-	if (result == ISC_R_NOMORE)
-		finished = true;
-	else if (result != ISC_R_SUCCESS)
+	if (result == ISC_R_NOMORE) {
+		atomic_store(&finished, true);
+	} else if (result != ISC_R_SUCCESS) {
 		fatal("failure iterating database: %s",
 		      isc_result_totext(result));
+	}
 }
 
 /*%
@@ -1478,11 +1481,12 @@ assignwork(isc_task_t *task, isc_task_t *worker) {
 	static dns_fixedname_t fzonecut;	/* Protected by namelock. */
 	static unsigned int ended = 0;		/* Protected by namelock. */
 
-	if (shuttingdown)
+	if (atomic_load(&shuttingdown)) {
 		return;
+	}
 
 	LOCK(&namelock);
-	if (finished) {
+	if (atomic_load(&finished)) {
 		ended++;
 		if (ended == ntasks) {
 			isc_task_detach(&task);
@@ -1552,7 +1556,7 @@ assignwork(isc_task_t *task, isc_task_t *worker) {
  next:
 		result = dns_dbiterator_next(gdbiter);
 		if (result == ISC_R_NOMORE) {
-			finished = true;
+			atomic_store(&finished, true);
 			break;
 		} else if (result != ISC_R_SUCCESS)
 			fatal("failure iterating database: %s",
@@ -3854,7 +3858,7 @@ main(int argc, char *argv[]) {
 	presign();
 	TIME_NOW(&sign_start);
 	signapex();
-	if (!finished) {
+	if (!atomic_load(&finished)) {
 		/*
 		 * There is more work to do.  Spread it out over multiple
 		 * processors if possible.
@@ -3867,11 +3871,12 @@ main(int argc, char *argv[]) {
 				      isc_result_totext(result));
 		}
 		(void)isc_app_run();
-		if (!finished)
+		if (!atomic_load(&finished)) {
 			fatal("process aborted by user");
+		}
 	} else
 		isc_task_detach(&master);
-	shuttingdown = true;
+	atomic_store(&shuttingdown, true);;
 	for (i = 0; i < (int)ntasks; i++)
 		isc_task_detach(&tasks[i]);
 	isc_taskmgr_destroy(&taskmgr);