diff -Nru haproxy-2.0.13/debian/changelog haproxy-2.0.13/debian/changelog --- haproxy-2.0.13/debian/changelog 2020-04-01 19:49:32.000000000 +0000 +++ haproxy-2.0.13/debian/changelog 2020-09-08 17:16:14.000000000 +0000 @@ -1,3 +1,13 @@ +haproxy (2.0.13-2ubuntu0.1) focal; urgency=medium + + * Backport dns related fixes from git to resolve crashes when + using do-resolve action (LP: #1894879) + - BUG/CRITICAL: dns: Make the do-resolve action thread safe + - BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed + - BUG/MEDIUM: dns: Don't yield in do resolve action on a final + + -- Simon Deziel Tue, 08 Sep 2020 17:16:14 +0000 + haproxy (2.0.13-2) unstable; urgency=medium * d/dconv: replace cgi.escape by html.escape. Closes: #951416. diff -Nru haproxy-2.0.13/debian/patches/0003-BUG-CRITICAL-dns-Make-the-do-resolve-action-thread-safe.patch haproxy-2.0.13/debian/patches/0003-BUG-CRITICAL-dns-Make-the-do-resolve-action-thread-safe.patch --- haproxy-2.0.13/debian/patches/0003-BUG-CRITICAL-dns-Make-the-do-resolve-action-thread-safe.patch 1970-01-01 00:00:00.000000000 +0000 +++ haproxy-2.0.13/debian/patches/0003-BUG-CRITICAL-dns-Make-the-do-resolve-action-thread-safe.patch 2020-09-08 17:13:38.000000000 +0000 @@ -0,0 +1,132 @@ +From ef131aee357478c45d547abcb0ab21c2a191f578 Mon Sep 17 00:00:00 2001 +From: Christopher Faulet +Date: Wed, 22 Jul 2020 11:46:32 +0200 +Subject: [PATCH] BUG/MAJOR: dns: Make the do-resolve action thread-safe + +The do-resolve HTTP action, performing a DNS resolution of a sample expression +output, is not thread-safe at all. The resolver object used to do the resolution +must be locked when the action is executed or when the stream is released +because its curr or wait resolution lists and the requester list inside a +resolution are updated. It is also important to not wake up a released stream +(with a destroyed task). + +Of course, because of this bug, various kind of crashes may be observed. + +This patch should fix the issue #236. It must be backported as far as 2.0. + +(cherry picked from commit 5098a08c2fafb0d9513996729d2a30c9785378f3) +Signed-off-by: Willy Tarreau +(cherry picked from commit 99f4623952cbbad2bcae451abdd0f3133bcbe75c) +Signed-off-by: Christopher Faulet +(cherry picked from commit 6e5861d72fe1e3c9d34b591d6f77ffd28ddde197) +Signed-off-by: Christopher Faulet +--- + src/dns.c | 36 +++++++++++++++++++++++++++--------- + src/stream.c | 4 ++++ + 2 files changed, 31 insertions(+), 9 deletions(-) + +diff --git a/src/dns.c b/src/dns.c +index 282fa92..40e29ad 100644 +--- a/src/dns.c ++++ b/src/dns.c +@@ -2162,14 +2162,23 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px, + struct dns_requester *req; + struct dns_resolvers *resolvers; + struct dns_resolution *res; +- int exp; ++ int exp, locked = 0; ++ enum act_return ret = ACT_RET_CONT; ++ ++ resolvers = rule->arg.dns.resolvers; + + /* we have a response to our DNS resolution */ + use_cache: + if (s->dns_ctx.dns_requester && s->dns_ctx.dns_requester->resolution != NULL) { + resolution = s->dns_ctx.dns_requester->resolution; ++ if (!locked) { ++ HA_SPIN_LOCK(DNS_LOCK, &resolvers->lock); ++ locked = 1; ++ } ++ + if (resolution->step == RSLV_STEP_RUNNING) { +- return ACT_RET_YIELD; ++ ret = ACT_RET_YIELD; ++ goto end; + } + if (resolution->step == RSLV_STEP_NONE) { + /* We update the variable only if we have a valid response. */ +@@ -2211,29 +2220,33 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px, + pool_free(dns_requester_pool, s->dns_ctx.dns_requester); + s->dns_ctx.dns_requester = NULL; + +- return ACT_RET_CONT; ++ goto end; + } + + /* need to configure and start a new DNS resolution */ + smp = sample_fetch_as_type(px, sess, s, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, rule->arg.dns.expr, SMP_T_STR); + if (smp == NULL) +- return ACT_RET_CONT; ++ goto end; + + fqdn = smp->data.u.str.area; + if (action_prepare_for_resolution(s, fqdn) == -1) +- return ACT_RET_ERR; ++ goto end; /* on error, ignore the action */ + + s->dns_ctx.parent = rule; ++ ++ HA_SPIN_LOCK(DNS_LOCK, &resolvers->lock); ++ locked = 1; ++ + dns_link_resolution(s, OBJ_TYPE_STREAM, 0); + + /* 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); +- return ACT_RET_YIELD; ++ ret = ACT_RET_YIELD; ++ goto end; + } +- res = req->resolution; +- resolvers = res->resolvers; ++ 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) +@@ -2242,7 +2255,12 @@ enum act_return dns_action_do_resolve(struct act_rule *rule, struct proxy *px, + } + + dns_trigger_resolution(s->dns_ctx.dns_requester); +- return ACT_RET_YIELD; ++ ret = ACT_RET_YIELD; ++ ++ end: ++ if (locked) ++ HA_SPIN_UNLOCK(DNS_LOCK, &resolvers->lock); ++ return ret; + } + + +diff --git a/src/stream.c b/src/stream.c +index 080189e..2eb7cfa 100644 +--- a/src/stream.c ++++ b/src/stream.c +@@ -435,9 +435,13 @@ static void stream_free(struct stream *s) + } + + if (s->dns_ctx.dns_requester) { ++ __decl_hathreads(struct dns_resolvers *resolvers = s->dns_ctx.parent->arg.dns.resolvers); ++ ++ HA_SPIN_LOCK(DNS_LOCK, &resolvers->lock); + 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); ++ HA_SPIN_UNLOCK(DNS_LOCK, &resolvers->lock); + + pool_free(dns_requester_pool, s->dns_ctx.dns_requester); + s->dns_ctx.dns_requester = NULL; +-- +1.7.10.4 + diff -Nru haproxy-2.0.13/debian/patches/0004-BUG-MEDIUM-dns-Release-answer-items-when-a-DNS-resolution-is-freed.patch haproxy-2.0.13/debian/patches/0004-BUG-MEDIUM-dns-Release-answer-items-when-a-DNS-resolution-is-freed.patch --- haproxy-2.0.13/debian/patches/0004-BUG-MEDIUM-dns-Release-answer-items-when-a-DNS-resolution-is-freed.patch 1970-01-01 00:00:00.000000000 +0000 +++ haproxy-2.0.13/debian/patches/0004-BUG-MEDIUM-dns-Release-answer-items-when-a-DNS-resolution-is-freed.patch 2020-09-08 17:14:48.000000000 +0000 @@ -0,0 +1,51 @@ +From 39eb766825d8aad09946dfc284d4a73f610ebd64 Mon Sep 17 00:00:00 2001 +From: Christopher Faulet +Date: Wed, 22 Jul 2020 15:55:49 +0200 +Subject: [PATCH] BUG/MEDIUM: dns: Release answer items when a DNS resolution + is freed + +When a DNS resolution is freed, the remaining items in .ar_list and .answer_list +are also released. It must be done to avoid a memory leak. And it is the last +chance to release these objects. I've honestly no idea if there is a better +place to release them earlier. But at least, there is no more leak. + +This patch should solve the issue #222. It must be backported, at least, as far +as 2.0, and probably, with caution, as far as 1.8 or 1.7. + +(cherry picked from commit 010ab35a9118daf17a670fb2b42e40447f967f7c) +Signed-off-by: Willy Tarreau +(cherry picked from commit c58ac80d00284886b108b209a5bf993de5ab38ed) +Signed-off-by: Christopher Faulet +(cherry picked from commit 81120e6ea286ae3f2566959167fb56a7d1f0de19) +Signed-off-by: Christopher Faulet +--- + src/dns.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/src/dns.c b/src/dns.c +index 40e29ad..18e64d9 100644 +--- a/src/dns.c ++++ b/src/dns.c +@@ -1336,6 +1336,7 @@ static struct dns_resolution *dns_pick_resolution(struct dns_resolvers *resolver + static void dns_free_resolution(struct dns_resolution *resolution) + { + struct dns_requester *req, *reqback; ++ struct dns_answer_item *item, *itemback; + + /* clean up configuration */ + dns_reset_resolution(resolution); +@@ -1347,6 +1348,11 @@ static void dns_free_resolution(struct dns_resolution *resolution) + req->resolution = NULL; + } + ++ list_for_each_entry_safe(item, itemback, &resolution->response.answer_list, list) { ++ LIST_DEL(&item->list); ++ pool_free(dns_answer_item_pool, item); ++ } ++ + LIST_DEL(&resolution->list); + pool_free(dns_resolution_pool, resolution); + } +-- +1.7.10.4 + diff -Nru haproxy-2.0.13/debian/patches/0005-BUG-MEDIUM-dns-Dont-yield-in-do-resolve-action-on-a-final.patch haproxy-2.0.13/debian/patches/0005-BUG-MEDIUM-dns-Dont-yield-in-do-resolve-action-on-a-final.patch --- haproxy-2.0.13/debian/patches/0005-BUG-MEDIUM-dns-Dont-yield-in-do-resolve-action-on-a-final.patch 1970-01-01 00:00:00.000000000 +0000 +++ haproxy-2.0.13/debian/patches/0005-BUG-MEDIUM-dns-Dont-yield-in-do-resolve-action-on-a-final.patch 2020-09-08 17:15:42.000000000 +0000 @@ -0,0 +1,106 @@ +From 74d704f2f36945d60f1ff7ea75dbfe3f40508861 Mon Sep 17 00:00:00 2001 +From: Christopher Faulet +Date: Tue, 28 Jul 2020 10:21:54 +0200 +Subject: [PATCH] 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. + +(cherry picked from commit 385101e53816dc1b7bc1fc957adc512ce8a07cb4) +Signed-off-by: Christopher Faulet +(cherry picked from commit 5c038f759959adf95b4b347aba9d97e60ab87e93) +Signed-off-by: Christopher Faulet +(cherry picked from commit af018c4865400dda4553a732df4c43751a4ff88c) +Signed-off-by: Christopher Faulet +--- + src/dns.c | 39 +++++++++++++++++++++------------------ + 1 file changed, 21 insertions(+), 18 deletions(-) + +diff --git a/src/dns.c b/src/dns.c +index 18e64d9..095040e 100644 +--- a/src/dns.c ++++ b/src/dns.c +@@ -2182,10 +2182,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) { +@@ -2219,14 +2217,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 */ +@@ -2247,26 +2238,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_FLAG_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; + } + + +-- +1.7.10.4 + diff -Nru haproxy-2.0.13/debian/patches/series haproxy-2.0.13/debian/patches/series --- haproxy-2.0.13/debian/patches/series 2020-04-01 19:49:32.000000000 +0000 +++ haproxy-2.0.13/debian/patches/series 2020-09-08 17:16:09.000000000 +0000 @@ -5,5 +5,9 @@ # 20200402 security issue (CVE-2020-11100) about HTTP/2 HPACK header table 0001-BUG-CRITICAL-hpack-never-index-a-header-into-the-hea.patch +0003-BUG-CRITICAL-dns-Make-the-do-resolve-action-thread-safe.patch +0004-BUG-MEDIUM-dns-Release-answer-items-when-a-DNS-resolution-is-freed.patch +0005-BUG-MEDIUM-dns-Dont-yield-in-do-resolve-action-on-a-final.patch + # applied during the build process: # debianize-dconv.patch