BUG/MEDIUM: resolvers: detach servers without resolvers from the SRV request

All servers of a proxy using the same SRV name share the same SRV request
(srvrq), and since commit 81ba74ae5 ("BUG/MEDIUM: resolvers: Make 1st
server of a template take part to SRV resolution") they are all attached
to its list of available servers while parsing their address. But the
"resolvers" keyword is only known later, and resolvers_finalize_config()
simply skips servers without it, leaving them in the list without their
srvrq_check task nor their resolvers. If another server of the same proxy
uses the same SRV name with a "resolvers" keyword, the SRV request is
resolved, and resolv_check_response() may pick the server without
resolvers when assigning SRV records, then crashes dereferencing its NULL
srvrq_check (and would call resolv_link_resolution() with a NULL
resolvers otherwise).

This is easily reproduced with such a backend, which is accepted without
any warning, and a nameserver answering two SRV records:

    backend be
        server x _http._tcp.svc.test init-addr none
        server-template a 2 _http._tcp.svc.test resolvers dns init-addr none

The process segfaults on the first SRV response. The reg-test provided
with this patch does exactly this.

But detaching the server from the SRV request's list of available servers
is not enough: the pointer to the SRV request itself must also be dropped.
Otherwise "show servers state" keeps dumping the SRV record name for this
server, and on a reload with a server state file, srv_apply_state() re-adds
the server to the SRV request's named_servers tree before crashing while
scheduling its NULL srvrq_check task. The server could also be picked from
that tree again by any subsequent SRV response. Note that a server with an
SRV name but no resolvers will never be resolved: keeping its pointer to
the SRV request serves no purpose.

Thus, in resolvers_finalize_config(), let's fully detach such servers
from the SRV request, since they will never be resolved anyway. As a
consequence, their state is no longer applied on reload (the SRV record
reported in the state file can no longer match), which is the desired
behavior, and it is no longer forbidden to change their FQDN from the
stats socket.

This must be backported to all stable branches.

diff --git a/src/resolvers.c b/src/resolvers.c
index a124c66e3d475ab2de58dff95075ec067b33b28c..bddb457c907590c320bd9356c3c4c145e88cd51b 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -2923,8 +2923,29 @@ static int resolvers_finalize_config(void)
                list_for_each_entry(srv, &px->servers, el_px) {
                        struct resolvers *resolvers;
 
-                       if (!srv->resolvers_id)
+                       if (!srv->resolvers_id) {
+                               /* A server with an SRV name but no resolvers
+                                * will never be resolved, yet it's attached to
+                                * the SRV request shared with other servers of
+                                * this proxy using the same name, which could
+                                * pick it during the assignment of the SRV
+                                * records (it would crash on its NULL
+                                * srvrq_check task). But the SRV request must
+                                * also be removed altogether: otherwise, on a
+                                * reload with a server state file, the server
+                                * could be re-added to the SRV request's
+                                * named_servers tree and crash on the same
+                                * NULL srvrq_check when scheduling its
+                                * expiration task, and any later SRV response
+                                * could also pick it from that tree. Let's
+                                * detach it and drop the reference.
+                                */
+                               if (srv->srvrq) {
+                                       LIST_DEL_INIT(&srv->srv_rec_item);
+                                       srv->srvrq = NULL;
+                               }
                                continue;
+                       }
 
                        if ((resolvers = find_resolvers_by_id(srv->resolvers_id)) == NULL) {
                                ha_alert("%s '%s', server '%s': unable to find required resolvers '%s'\n",