]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
authorChristopher Faulet <cfaulet@haproxy.com>
Tue, 28 Jul 2020 08:21:54 +0000 (10:21 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Thu, 30 Jul 2020 07:31:09 +0000 (09:31 +0200)
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.

src/dns.c

index f5bce4261497b041186d44879f0de1a0214bd3ad..c8f34874d00489572194024cea329b62b8628454 100644 (file)
--- 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)