Hi,

The recently merged MT code broke SRV records by attempting to recursively
lock the DNS lock. The attached patch attemps to fix this by letting the
relevant code know if the lock is already held or not.

Regards,

Olivier
>From 4df4df3bc00ddf9a1e7b94188058a0d700006816 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Tue, 31 Oct 2017 15:21:19 +0100
Subject: [PATCH] BUG/MINOR: dns: Fix SRV records with the new thread code.

srv_set_fqdn() may be called with the DNS lock already held, but tries to
lock it anyway. So, add a new parameter to let it know if it was already
locked or not;
---
 include/proto/server.h |  2 +-
 src/dns.c              |  2 +-
 src/server.c           | 21 ++++++++++++---------
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/proto/server.h b/include/proto/server.h
index ff4ec77ca..14f4926e2 100644
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -56,7 +56,7 @@ extern struct list updated_servers;
 
 /* functions related to server name resolution */
 int snr_update_srv_status(struct server *s, int has_no_ip);
-const char *update_server_fqdn(struct server *server, const char *fqdn, const 
char *updater);
+const char *update_server_fqdn(struct server *server, const char *fqdn, const 
char *updater, int dns_locked);
 int snr_resolution_cb(struct dns_requester *requester, struct dns_nameserver 
*nameserver);
 int snr_resolution_error_cb(struct dns_requester *requester, int error_code);
 struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned 
char *ip_family);
diff --git a/src/dns.c b/src/dns.c
index 1fdc461e6..a8d468cf4 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -545,7 +545,7 @@ static void dns_check_dns_response(struct dns_resolution 
*res)
                                if (dns_dn_label_to_str(item->target, 
item->data_len+1,
                                                        hostname, 
DNS_MAX_NAME_SIZE) == -1)
                                        continue;
-                               msg = update_server_fqdn(srv, hostname, "SRV 
record");
+                               msg = update_server_fqdn(srv, hostname, "SRV 
record", 1);
                                if (msg)
                                        send_log(srv->proxy, LOG_NOTICE, "%s", 
msg);
 
diff --git a/src/server.c b/src/server.c
index 2d0e3b4f9..37f90d8c9 100644
--- a/src/server.c
+++ b/src/server.c
@@ -53,7 +53,7 @@ HA_SPINLOCK_T updated_servers_lock;
 static void srv_register_update(struct server *srv);
 static void srv_update_state(struct server *srv, int version, char **params);
 static int srv_apply_lastaddr(struct server *srv, int *err_code);
-static int srv_set_fqdn(struct server *srv, const char *fqdn);
+static int srv_set_fqdn(struct server *srv, const char *fqdn, int dns_locked);
 
 /* List head of all known server keywords */
 static struct srv_kw_list srv_keywords = {
@@ -2911,7 +2911,7 @@ static void srv_update_state(struct server *srv, int 
version, char **params)
                                         * from stats socket).
                                         */
                                        if (fqdn_set_by_cli) {
-                                               srv_set_fqdn(srv, fqdn);
+                                               srv_set_fqdn(srv, fqdn, 0);
                                                srv->next_admin |= 
SRV_ADMF_HMAINT;
                                        }
                                }
@@ -3764,13 +3764,14 @@ int srv_set_addr_via_libc(struct server *srv, int 
*err_code)
 /* Set the server's FDQN (->hostname) from <hostname>.
  * Returns -1 if failed, 0 if not.
  */
-int srv_set_fqdn(struct server *srv, const char *hostname)
+int srv_set_fqdn(struct server *srv, const char *hostname, int dns_locked)
 {
        struct dns_resolution *resolution;
        char                  *hostname_dn;
        int                    hostname_len, hostname_dn_len;
 
-       SPIN_LOCK(DNS_LOCK, &srv->resolvers->lock);
+       if (!dns_locked)
+               SPIN_LOCK(DNS_LOCK, &srv->resolvers->lock);
        /* run time DNS resolution was not active for this server
         * and we can't enable it at run time for now.
         */
@@ -3805,11 +3806,13 @@ int srv_set_fqdn(struct server *srv, const char 
*hostname)
                goto err;
 
   end:
-       SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock);
+       if (!dns_locked)
+               SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock);
        return 0;
 
   err:
-       SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock);
+       if (!dns_locked)
+               SPIN_UNLOCK(DNS_LOCK, &srv->resolvers->lock);
        return -1;
 }
 
@@ -3936,7 +3939,7 @@ int srv_init_addr(void)
        return return_code;
 }
 
-const char *update_server_fqdn(struct server *server, const char *fqdn, const 
char *updater)
+const char *update_server_fqdn(struct server *server, const char *fqdn, const 
char *updater, int dns_locked)
 {
 
        struct chunk *msg;
@@ -3957,7 +3960,7 @@ const char *update_server_fqdn(struct server *server, 
const char *fqdn, const ch
        chunk_appendf(msg, "%s/%s changed its FQDN from %s to %s",
                      server->proxy->id, server->id, server->hostname, fqdn);
 
-       if (srv_set_fqdn(server, fqdn) < 0) {
+       if (srv_set_fqdn(server, fqdn, dns_locked) < 0) {
                chunk_reset(msg);
                chunk_appendf(msg, "could not update %s/%s FQDN",
                              server->proxy->id, server->id);
@@ -4185,7 +4188,7 @@ static int cli_parse_set_server(char **args, struct 
appctx *appctx, void *privat
                        appctx->st0 = CLI_ST_PRINT;
                        return 1;
                }
-               warning = update_server_fqdn(sv, args[4], "stats socket 
command");
+               warning = update_server_fqdn(sv, args[4], "stats socket 
command", 0);
                if (warning) {
                        appctx->ctx.cli.severity = LOG_WARNING;
                        appctx->ctx.cli.msg = warning;
-- 
2.13.5

Reply via email to