From be6cc53ec27e4549bf166d2161bacce688151e79 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Tue, 23 Jun 2020 13:02:21 +0200
Subject: [PATCH] Don't continue opening a new rndc connection if we are
 shutting down

Due to lack of synchronization, whenever named was being requested to
stop using rndc, controlconf.c module could be trying to access an already
released pointer through named_g_server->interfacemgr in a separate
thread.

The race could only be triggered if named was being shutdown and more
rndc connections were ocurring at the same time.

This fix correctly checks if the server is shutting down before opening
a new rndc connection.
---
 bin/named/controlconf.c | 66 +++++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c
index a3fda486d3..bab50ae8e0 100644
--- a/bin/named/controlconf.c
+++ b/bin/named/controlconf.c
@@ -226,6 +226,7 @@ shutdown_listener(controllistener_t *listener) {
 	if (listener->listening) {
 		isc_socket_cancel(listener->sock, listener->task,
 				  ISC_SOCKCANCEL_ACCEPT);
+		return;
 	}
 
 	maybe_free_listener(listener);
@@ -636,6 +637,8 @@ control_newconn(isc_task_t *task, isc_event_t *event) {
 
 	UNUSED(task);
 
+	REQUIRE(listener->listening);
+
 	listener->listening = false;
 
 	if (nevent->result != ISC_R_SUCCESS) {
@@ -647,6 +650,14 @@ control_newconn(isc_task_t *task, isc_event_t *event) {
 	}
 
 	sock = nevent->newsocket;
+
+	/* Is the server shutting down? */
+	if (listener->controls->shuttingdown) {
+		isc_socket_detach(&sock);
+		shutdown_listener(listener);
+		goto cleanup;
+	}
+
 	isc_socket_setname(sock, "control", NULL);
 	(void)isc_socket_getpeername(sock, &peeraddr);
 	if (listener->type == isc_sockettype_tcp &&
@@ -1121,36 +1132,33 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp,
 
 	listener = isc_mem_get(mctx, sizeof(*listener));
 
-	if (result == ISC_R_SUCCESS) {
-		listener->mctx = NULL;
-		isc_mem_attach(mctx, &listener->mctx);
-		listener->controls = cp;
-		listener->task = cp->server->task;
-		listener->address = *addr;
-		listener->sock = NULL;
-		listener->listening = false;
-		listener->exiting = false;
-		listener->acl = NULL;
-		listener->type = type;
-		listener->perm = 0;
-		listener->owner = 0;
-		listener->group = 0;
-		listener->readonly = false;
-		ISC_LINK_INIT(listener, link);
-		ISC_LIST_INIT(listener->keys);
-		ISC_LIST_INIT(listener->connections);
+	listener->mctx = NULL;
+	isc_mem_attach(mctx, &listener->mctx);
+	listener->controls = cp;
+	listener->task = cp->server->task;
+	listener->address = *addr;
+	listener->sock = NULL;
+	listener->listening = false;
+	listener->exiting = false;
+	listener->acl = NULL;
+	listener->type = type;
+	listener->perm = 0;
+	listener->owner = 0;
+	listener->group = 0;
+	listener->readonly = false;
+	ISC_LINK_INIT(listener, link);
+	ISC_LIST_INIT(listener->keys);
+	ISC_LIST_INIT(listener->connections);
 
-		/*
-		 * Make the acl.
-		 */
-		if (control != NULL && type == isc_sockettype_tcp) {
-			allow = cfg_tuple_get(control, "allow");
-			result = cfg_acl_fromconfig(allow, config, named_g_lctx,
-						    aclconfctx, mctx, 0,
-						    &new_acl);
-		} else {
-			result = dns_acl_any(mctx, &new_acl);
-		}
+	/*
+	 * Make the acl.
+	 */
+	if (control != NULL && type == isc_sockettype_tcp) {
+		allow = cfg_tuple_get(control, "allow");
+		result = cfg_acl_fromconfig(allow, config, named_g_lctx,
+					    aclconfctx, mctx, 0, &new_acl);
+	} else {
+		result = dns_acl_any(mctx, &new_acl);
 	}
 
 	if ((result == ISC_R_SUCCESS) && (control != NULL)) {