From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Tue, 12 Dec 2023 18:47:56 +0000 (+0100)
Subject: BUG/MEDIUM: server/dns: perform svc_port updates atomically from SRV records
X-Git-Tag: v3.0-dev1~52
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c5cace310013529389b22a4307869caa79ec395f;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: server/dns: perform svc_port updates atomically from SRV records

This was the last missing bit from cd994407a ("BUG/MAJOR: server/addr:
fix a race during server addr:svc_port updates")

Indeed, despite the fix, svc_port updates from resolvers were still
directly performed on the server's struct.

Now they make proper use of the server_set_inetaddr() function so the port
change (+ optional addr change with AR) will be propagated atomically.

This patch depends on:
 - "MINOR: server: ensure connection cleanup on server addr changes"
 - "CLEANUP: server/event_hdl: remove purge_conn hint in INETADDR event"
 - "MEDIUM: server: merge srv_update_addr() and srv_update_addr_port() logic"
 - "MEDIUM: server: make server_set_inetaddr() updater serializable"
 - "MINOR: server/event_hdl: expose updater info through INETADDR event"
 - "MINOR: server: add dns hint in server_inetaddr_updater struct"
 - "MEDIUM: server/dns: clear RMAINT when addr resolves again"

While it could be backported in 2.9 with cd994407a ("BUG/MAJOR:
server/addr: fix a race during server addr:svc_port updates") to ensure
addr and svc_port updates performed by resolver's code comply with the
API taking care of pushing the update (and thus avoid any race), some
patch dependencies are quite sensitive so it's probably best to avoid
backporting for no good reason, or at least wait for it to be considered
stable to prevent any breakeages
---

diff --git a/src/resolvers.c b/src/resolvers.c
index ab9541467e..96af44a0c9 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -832,12 +832,16 @@ static void resolv_check_response(struct resolv_resolution *res)
 srv_found:
 			/* And update this server, if found (srv is locked here) */
 			if (srv) {
+				struct server_inetaddr srv_addr;
+				uint8_t ip_change = 0;
+
 				/* re-enable DNS resolution for this server by default */
 				srv->flags &= ~SRV_F_NO_RESOLUTION;
 				srv->srvrq_check->expire = TICK_ETERNITY;
 
-				srv->svc_port = item->port;
-				srv->flags   &= ~SRV_F_MAPPORTS;
+				server_get_inetaddr(srv, &srv_addr);
+				srv_addr.port.svc = item->port;
+				srv_addr.port.map = 0;
 
 				/* Check if an Additional Record is associated to this SRV record.
 				 * Perform some sanity checks too to ensure the record can be used.
@@ -850,10 +854,12 @@ srv_found:
 
 					switch (item->ar_item->type) {
 						case DNS_RTYPE_A:
-							srv_update_addr(srv, &item->ar_item->data.in4.sin_addr, AF_INET, SERVER_INETADDR_UPDATER_DNS_AR);
+							srv_addr.family = AF_INET;
+							srv_addr.addr.v4 = item->ar_item->data.in4.sin_addr;
 						break;
 						case DNS_RTYPE_AAAA:
-							srv_update_addr(srv, &item->ar_item->data.in6.sin6_addr, AF_INET6, SERVER_INETADDR_UPDATER_DNS_AR);
+							srv_addr.family = AF_INET6;
+							srv_addr.addr.v6 = item->ar_item->data.in6.sin6_addr;
 						break;
 					}
 
@@ -863,8 +869,15 @@ srv_found:
 					 * It is usless to perform an extra resolution
 					 */
 					_resolv_unlink_resolution(srv->resolv_requester);
+
+					ip_change = 1;
 				}
 
+				if (ip_change)
+					server_set_inetaddr_warn(srv, &srv_addr, SERVER_INETADDR_UPDATER_DNS_AR);
+				else
+					server_set_inetaddr(srv, &srv_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
+
 				if (!srv->hostname_dn) {
 					const char *msg = NULL;
 					char hostname[DNS_MAX_NAME_SIZE+1];