From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 28 Jul 2020 08:21:54 +0000 (+0200)
Subject: BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
X-Git-Tag: v2.3-dev2~21
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=385101e53816dc1b7bc1fc957adc512ce8a07cb4;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation

When an action is evaluated, flags are passed to know if it is the first call
(ACT_OPT_FIRST) and if it must be the last one (ACT_OPT_FINAL). For the
do-resolve DNS action, the ACT_OPT_FINAL flag must be handled because the
action may yield. It must never yield when this flag is set. Otherwise, it may
lead to a wakeup loop of the stream because the inspected-delay of a tcp-request
content ruleset was reached without stopping the rules evaluation.

This patch is related to the issue #222. It must be backported as far as 2.0.
---

diff --git a/src/dns.c b/src/dns.c
index f5bce42614..c8f34874d0 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -2456,10 +2456,8 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
 			locked = 1;
 		}
 
-		if (resolution->step == RSLV_STEP_RUNNING) {
-			ret = ACT_RET_YIELD;
-			goto end;
-		}
+		if (resolution->step == RSLV_STEP_RUNNING)
+			goto yield;
 		if (resolution->step == RSLV_STEP_NONE) {
 			/* We update the variable only if we have a valid response. */
 			if (resolution->status == RSLV_STATUS_VALID) {
@@ -2493,14 +2491,7 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
 			}
 		}
 
-		free(s->dns_ctx.hostname_dn); s->dns_ctx.hostname_dn = NULL;
-		s->dns_ctx.hostname_dn_len = 0;
-		dns_unlink_resolution(s->dns_ctx.dns_requester);
-
-		pool_free(dns_requester_pool, s->dns_ctx.dns_requester);
-		s->dns_ctx.dns_requester = NULL;
-
-		goto end;
+		goto release_requester;
 	}
 
 	/* need to configure and start a new DNS resolution */
@@ -2521,26 +2512,38 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px,
 
 	/* Check if there is a fresh enough response in the cache of our associated resolution */
 	req = s->dns_ctx.dns_requester;
-	if (!req || !req->resolution) {
-		dns_trigger_resolution(s->dns_ctx.dns_requester);
-		ret = ACT_RET_YIELD;
-		goto end;
-	}
+	if (!req || !req->resolution)
+		goto release_requester; /* on error, ignore the action */
 	res = req->resolution;
 
 	exp = tick_add(res->last_resolution, resolvers->hold.valid);
 	if (resolvers->t && res->status == RSLV_STATUS_VALID && tick_isset(res->last_resolution)
-		       && !tick_is_expired(exp, now_ms)) {
+	    && !tick_is_expired(exp, now_ms)) {
 		goto use_cache;
 	}
 
 	dns_trigger_resolution(s->dns_ctx.dns_requester);
+
+  yield:
+	if (flags & ACT_OPT_FINAL)
+		goto release_requester;
 	ret = ACT_RET_YIELD;
 
   end:
 	if (locked)
 		HA_SPIN_UNLOCK(DNS_LOCK, &resolvers->lock);
 	return ret;
+
+  release_requester:
+	free(s->dns_ctx.hostname_dn);
+	s->dns_ctx.hostname_dn = NULL;
+	s->dns_ctx.hostname_dn_len = 0;
+	if (s->dns_ctx.dns_requester) {
+		dns_unlink_resolution(s->dns_ctx.dns_requester);
+		pool_free(dns_requester_pool, s->dns_ctx.dns_requester);
+		s->dns_ctx.dns_requester = NULL;
+	}
+	goto end;
 }
 
 static void release_dns_action(struct act_rule *rule)