On Mon, Jan 04, 2021 at 12:58:25AM +0100, Tim Duesterhus wrote:
> I am adding Thayne as CC as it was your commit that uncovered the issue and
> the
> crash happened in a function you wrote. Maybe you might want to add some
> additional checks somewhere?
Oops, my bad. I actually meant to put a NULL check in there, but must have
forgotten.
On 1/5/21 3:37 AM, Willy Tarreau wrote:
> Unfortunately we cannot do that or it destroys usage of the DNS or
> even any runtime address assignment over the CLI. For example if you
> write:
>
> server foo foo init-addr none
>
> I guess it won't work anymore since that's placed just after the address
> parsing. This means however that there probably is something more
> problematic in the referencing of address-less servers. Either servers
> are added/updated each time their address changes (to follow DNS) and
> then we can simply skip their registration when they're address-less.
> Or the servers are only registered at boot time, and the synchronization
> will fail after any address update. What do you think, Thayne ?
I'm partial to skipping registration when they are address-less.
Here's a patch that I think should fix this:
-- >8 --
Subject: [PATCH] Fix a segfault in srv_set_addr_desc
Check to make sure the address key is non-null before using it for
comparison or inserting it into the tree.
---
src/server.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/server.c b/src/server.c
index 50c6da131..d1aa51dc8 100644
--- a/src/server.c
+++ b/src/server.c
@@ -204,7 +204,7 @@ static void srv_set_addr_desc(struct server *s)
key = sa2str(&s->addr, s->svc_port, s->flags & SRV_F_MAPPORTS);
if (s->addr_node.key) {
- if (strcmp(key, s->addr_node.key) == 0) {
+ if (key && strcmp(key, s->addr_node.key) == 0) {
free(key);
return;
}
@@ -218,9 +218,11 @@ static void srv_set_addr_desc(struct server *s)
s->addr_node.key = key;
- HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
- ebis_insert(&p->used_server_addr, &s->addr_node);
- HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
+ if (s->addr_node.key) {
+ HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock);
+ ebis_insert(&p->used_server_addr, &s->addr_node);
+ HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &p->lock);
+ }
}
/*
--
2.30.0
