commit 9829010: [Fix] Unschedule DNS request when clearing IO channel

[Vsevolod Stakhov]

Author: Vsevolod Stakhov
Date: 2022-01-22 12:29:51 +0000
URL: https://github.com/rspamd/rspamd/commit/982901084d95e6a9a1b955d91ef7cf20ea72c673 (HEAD -> master)

[Fix] Unschedule DNS request when clearing IO channel

---
 contrib/librdns/resolver.c |  8 ++++----
 contrib/librdns/util.c     | 42 ++++++++++++++++++++++++++++--------------
 contrib/librdns/util.h     |  2 +-
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/contrib/librdns/resolver.c b/contrib/librdns/resolver.c
index 9f5ca9872..c12180ec1 100644
--- a/contrib/librdns/resolver.c
+++ b/contrib/librdns/resolver.c
@@ -549,7 +549,7 @@ rdns_process_udp_read (int fd, struct rdns_io_channel *ioc)
 						req->resolver->ups->data);
 			}
 
-			rdns_request_unschedule (req);
+			rdns_request_unschedule (req, true);
 
 			if (!(rep->flags & RDNS_TRUNCATED)) {
 				req->state = RDNS_REQUEST_REPLIED;
@@ -621,7 +621,7 @@ rdns_process_timer (void *arg)
 
 	if (req->state == RDNS_REQUEST_TCP) {
 		rep = rdns_make_reply (req, RDNS_RC_TIMEOUT);
-		rdns_request_unschedule (req);
+		rdns_request_unschedule (req, true);
 		req->state = RDNS_REQUEST_REPLIED;
 		req->func (rep, req->arg);
 		REF_RELEASE (req);
@@ -632,7 +632,7 @@ rdns_process_timer (void *arg)
 	if (req->retransmits == 0) {
 
 		rep = rdns_make_reply (req, RDNS_RC_TIMEOUT);
-		rdns_request_unschedule (req);
+		rdns_request_unschedule (req, true);
 		req->state = RDNS_REQUEST_REPLIED;
 		req->func (rep, req->arg);
 		REF_RELEASE (req);
@@ -655,7 +655,7 @@ rdns_process_timer (void *arg)
 		if (!IS_CHANNEL_ACTIVE(req->io) || cnt > 1) {
 			/* Do not reschedule IO requests on inactive sockets */
 			rdns_debug ("reschedule request with id: %d", (int)req->id);
-			rdns_request_unschedule (req);
+			rdns_request_unschedule (req, true);
 			REF_RELEASE (req->io);
 
 			if (resolver->ups) {
diff --git a/contrib/librdns/util.c b/contrib/librdns/util.c
index 25a7ab72f..f4c442563 100644
--- a/contrib/librdns/util.c
+++ b/contrib/librdns/util.c
@@ -633,25 +633,31 @@ rdns_request_retain (struct rdns_request *req)
 }
 
 void
-rdns_request_unschedule (struct rdns_request *req)
+rdns_request_unschedule (struct rdns_request *req, bool remove_from_hash)
 {
 	if (req->async_event) {
 		if (req->state == RDNS_REQUEST_WAIT_REPLY) {
 			req->async->del_timer (req->async->data,
 					req->async_event);
-			rdns_request_remove_from_hash(req);
+			if (remove_from_hash) {
+				rdns_request_remove_from_hash(req);
+			}
 			req->async_event = NULL;
 		}
 		else if (req->state == RDNS_REQUEST_WAIT_SEND) {
 			req->async->del_write (req->async->data,
 					req->async_event);
 			/* Remove from id hashes */
-			rdns_request_remove_from_hash(req);
+			if (remove_from_hash) {
+				rdns_request_remove_from_hash(req);
+			}
 			req->async_event = NULL;
 		}
 	}
 	else if (req->state == RDNS_REQUEST_TCP) {
-		rdns_request_remove_from_hash(req);
+		if (remove_from_hash) {
+			rdns_request_remove_from_hash(req);
+		}
 
 		req->async->del_timer(req->async->data,
 				req->async_event);
@@ -663,7 +669,7 @@ rdns_request_unschedule (struct rdns_request *req)
 void
 rdns_request_release (struct rdns_request *req)
 {
-	rdns_request_unschedule (req);
+	rdns_request_unschedule (req, true);
 	REF_RELEASE (req);
 }
 
@@ -701,24 +707,32 @@ rdns_ioc_tcp_reset (struct rdns_io_channel *ioc)
 		ioc->flags &= ~RDNS_CHANNEL_CONNECTED;
 	}
 
-	if (ioc->sock != -1) {
-		close (ioc->sock);
-		ioc->sock = -1;
-	}
-	if (ioc->saddr) {
-		free (ioc->saddr);
-		ioc->saddr = NULL;
-	}
-
 	/* Remove all requests pending as we are unable to complete them */
 	struct rdns_request *req;
 	kh_foreach_value(ioc->requests, req, {
 		struct rdns_reply *rep = rdns_make_reply (req, RDNS_RC_NETERR);
+		/*
+		 * Unschedule request explicitly as we set state to RDNS_REQUEST_REPLIED
+		 * that will prevent timer from being removed on req dtor.
+		 *
+		 * We skip hash removal here, as the hash will be cleared as a single
+		 * operation afterwards.
+		 */
+		rdns_request_unschedule(req, false);
 		req->state = RDNS_REQUEST_REPLIED;
 		req->func (rep, req->arg);
 		REF_RELEASE (req);
 	});
 
+	if (ioc->sock != -1) {
+		close (ioc->sock);
+		ioc->sock = -1;
+	}
+	if (ioc->saddr) {
+		free (ioc->saddr);
+		ioc->saddr = NULL;
+	}
+
 	kh_clear(rdns_requests_hash, ioc->requests);
 }
 
diff --git a/contrib/librdns/util.h b/contrib/librdns/util.h
index 915b8febd..6f74d8b85 100644
--- a/contrib/librdns/util.h
+++ b/contrib/librdns/util.h
@@ -94,6 +94,6 @@ struct rdns_reply * rdns_make_reply (struct rdns_request *req, enum dns_rcode rc
  */
 void rdns_reply_free (struct rdns_reply *rep);
 
-void rdns_request_unschedule (struct rdns_request *req);
+void rdns_request_unschedule (struct rdns_request *req, bool remove_from_hash);
 
 #endif /* UTIL_H_ */