]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: dns: fix missing lock protection on server.
authorEmeric Brun <ebrun@haproxy.com>
Thu, 2 Nov 2017 16:20:39 +0000 (17:20 +0100)
committerWilly Tarreau <w@1wt.eu>
Fri, 3 Nov 2017 14:17:55 +0000 (15:17 +0100)
To avoid inconsistencies server's attributes must be read
or updated under lock.

src/dns.c
src/server.c

index a8d468cf4dd3c1c94c5dcf602db00bf698e94c55..c1f3beef5067bd845989da154ea0a5ffef7bee3d 100644 (file)
--- a/src/dns.c
+++ b/src/dns.c
@@ -486,6 +486,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
 
                                /* Remove any associated server */
                                for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) {
+                                       SPIN_LOCK(SERVER_LOCK, &srv->lock);
                                        if (srv->srvrq == srvrq && srv->svc_port == item->port &&
                                            item->data_len == srv->hostname_dn_len &&
                                            !memcmp(srv->hostname_dn, item->target, item->data_len)) {
@@ -497,6 +498,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
                                                srv->hostname_dn_len = 0;
                                                dns_unlink_resolution(srv->dns_requester);
                                        }
+                                       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                                }
                        }
 
@@ -516,6 +518,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
 
                        /* Check if a server already uses that hostname */
                        for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) {
+                               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                                if (srv->srvrq == srvrq && srv->svc_port == item->port &&
                                    item->data_len == srv->hostname_dn_len &&
                                    !memcmp(srv->hostname_dn, item->target, item->data_len)) {
@@ -525,16 +528,20 @@ static void dns_check_dns_response(struct dns_resolution *res)
                                                snprintf(weight, sizeof(weight), "%d", item->weight);
                                                server_parse_weight_change_request(srv, weight);
                                        }
+                                       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                                        break;
                                }
+                               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                        }
                        if (srv)
                                continue;
 
                        /* If not, try to find a server with undefined hostname */
                        for (srv = srvrq->proxy->srv; srv != NULL; srv = srv->next) {
+                               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                                if (srv->srvrq == srvrq && !srv->hostname_dn)
                                        break;
+                               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                        }
                        /* And update this server, if found */
                        if (srv) {
@@ -556,6 +563,7 @@ static void dns_check_dns_response(struct dns_resolution *res)
                                        srv->check.port = item->port;
                                snprintf(weight, sizeof(weight), "%d", item->weight);
                                server_parse_weight_change_request(srv, weight);
+                               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                        }
                }
        }
@@ -1337,6 +1345,7 @@ int dns_link_resolution(void *requester, int requester_type)
                goto err;
 
        if (srv) {
+               SPIN_LOCK(SERVER_LOCK, &srv->lock);
                if (srv->dns_requester == NULL) {
                        if ((req = calloc(1, sizeof(*req))) == NULL)
                                goto err;
@@ -1345,6 +1354,7 @@ int dns_link_resolution(void *requester, int requester_type)
                }
                else
                        req = srv->dns_requester;
+               SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
        }
        else if (srvrq) {
                if (srvrq->dns_requester == NULL) {
index 37f90d8c9238d3bf739239fd15235c94ba902f39..555d6da19da712a3771db7d088ab3971b01f8f7b 100644 (file)
@@ -3712,8 +3712,14 @@ struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char
        if (!srv)
                return NULL;
 
+       SPIN_LOCK(SERVER_LOCK, &srv->lock);
+
        be = srv->proxy;
        for (tmpsrv = be->srv; tmpsrv; tmpsrv = tmpsrv->next) {
+               /* we found the current server is the same, ignore it */
+               if (srv == tmpsrv)
+                       continue;
+
                /* We want to compare the IP in the record with the IP of the servers in the
                 * same backend, only if:
                 *   * DNS resolution is enabled on the server
@@ -3721,15 +3727,20 @@ struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char
                 *     one used for the server found in the backend
                 *   * the server found in the backend is not our current server
                 */
+               SPIN_LOCK(SERVER_LOCK, &tmpsrv->lock);
                if ((tmpsrv->hostname_dn == NULL) ||
                    (srv->hostname_dn_len != tmpsrv->hostname_dn_len) ||
                    (strcmp(srv->hostname_dn, tmpsrv->hostname_dn) != 0) ||
-                   (srv->puid == tmpsrv->puid))
+                   (srv->puid == tmpsrv->puid)) {
+                       SPIN_UNLOCK(SERVER_LOCK, &tmpsrv->lock);
                        continue;
+               }
 
                /* If the server has been taken down, don't consider it */
-               if (tmpsrv->next_admin & SRV_ADMF_RMAINT)
+               if (tmpsrv->next_admin & SRV_ADMF_RMAINT) {
+                       SPIN_UNLOCK(SERVER_LOCK, &tmpsrv->lock);
                        continue;
+               }
 
                /* At this point, we have 2 different servers using the same DNS hostname
                 * for their respective resolution.
@@ -3739,10 +3750,15 @@ struct server *snr_check_ip_callback(struct server *srv, void *ip, unsigned char
                      memcmp(ip, &((struct sockaddr_in *)&tmpsrv->addr)->sin_addr, 4) == 0) ||
                     (tmpsrv->addr.ss_family == AF_INET6 &&
                      memcmp(ip, &((struct sockaddr_in6 *)&tmpsrv->addr)->sin6_addr, 16) == 0))) {
+                       SPIN_UNLOCK(SERVER_LOCK, &tmpsrv->lock);
+                       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                        return tmpsrv;
                }
+               SPIN_UNLOCK(SERVER_LOCK, &tmpsrv->lock);
        }
 
+       SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+
        return NULL;
 }