kozorizki/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: server/dns: use server_set_inetaddr() to unset srv addr from DNS

Browse Source 
As seen before, server's addr and svc_port should not be updated directly
during runtime, because even if the update is performed under the lock,
some competing threads might be reading ->addr and ->svc_port without
the lock because they simply cannot afford it.

To prevent races with such competing threads, server's addr and port
should only be updated using server_set_inetaddr() function or similar.

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 reset 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.
This commit is contained in:
Aurelien DARRAGON 2023-12-11 17:31:31 +01:00 committed by Christopher Faulet
parent 334ebfa1a2
commit 64c9c8ef39
2 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/resolvers.c
View File

@ -659,14 +659,17 @@ static void leave_resolver_code()
*/
static void resolv_srvrq_cleanup_srv(struct server *srv)
{
struct server_inetaddr srv_addr;
_resolv_unlink_resolution(srv->resolv_requester);
HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
srvrq_set_srv_down(srv);
ha_free(&srv->hostname);
ha_free(&srv->hostname_dn);
srv->hostname_dn_len = 0;
memset(&srv->addr, 0, sizeof(srv->addr));
srv->svc_port = 0;
memset(&srv_addr, 0, sizeof(srv_addr));
/* unset server's addr */
server_set_inetaddr(srv, &srv_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
srv->flags |= SRV_F_NO_RESOLUTION;
ebpt_delete(&srv->host_dn);

24
src/server.c
View File

@ -4448,8 +4448,13 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
srv_update_addr(s, firstip, firstip_sin_family, SERVER_INETADDR_UPDATER_DNS_CACHE);
update_status:
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip)
memset(&s->addr, 0, sizeof(s->addr));
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) {
struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr));
/* unset server's addr */
server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
}
return 1;
invalid:
@ -4457,8 +4462,13 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
counters->app.resolver.invalid++;
goto update_status;
}
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip)
memset(&s->addr, 0, sizeof(s->addr));
if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) {
struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr));
/* unset server's addr */
server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
}
return 0;
}
@ -4539,7 +4549,11 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code)
HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
if (!snr_set_srv_down(s, 1)) {
memset(&s->addr, 0, sizeof(s->addr));
struct server_inetaddr s_addr;
memset(&s_addr, 0, sizeof(s_addr));
/* unset server's addr */
server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
resolv_detach_from_resolution_answer_items(requester->resolution, requester);
return 0;