From d6afb53bdc252ec49d445f2530cad4e79fd4be57 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 9 Oct 2020 10:35:40 +0200 Subject: [PATCH] MEDIUM: listeners: always close master vs worker listeners Right now in enable_listener(), we used to start all enabled listeners then kill from the workers those that were for the master. But this is incomplete. We must also close from the master the listeners that are solely for workers, and do it before we even start them. Otherwise we end up with a master responding to the worker CLI connections if the listener remains in listen mode to translate the socket's real state. It doesn't seem like it could have caused bugs in the past because we used to aggressively mark disabled listeners as LI_ASSIGNED despite the fact that they were still bound and listening. If this patch were ever seen as a candidate solution for any obscure bug, be careful in that it subtly relies on the fact that fd_delete() doesn't close inherited FDs anymore, otherwise that could break the master's ability to pass inherited FDs on reloads. --- src/listener.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/listener.c b/src/listener.c index 48fdc96bb..364d061ac 100644 --- a/src/listener.c +++ b/src/listener.c @@ -282,6 +282,16 @@ void listener_set_state(struct listener *l, enum li_state st) void enable_listener(struct listener *listener) { HA_SPIN_LOCK(LISTENER_LOCK, &listener->lock); + + /* If this listener is supposed to be only in the master, close it in + * the workers. Conversely, if it's supposed to be only in the workers + * close it in the master. + */ + if ((master && !(listener->options & LI_O_MWORKER)) || + (!master && (listener->options & LI_O_MWORKER))) { + do_unbind_listener(listener, 1); + } + if (listener->state == LI_LISTEN) { BUG_ON(listener->rx.fd == -1); if ((global.mode & (MODE_DAEMON | MODE_MWORKER)) && @@ -304,12 +314,7 @@ void enable_listener(struct listener *listener) listener_set_state(listener, LI_FULL); } } - /* if this listener is supposed to be only in the master, close it in the workers */ - if ((global.mode & MODE_MWORKER) && - (listener->options & LI_O_MWORKER) && - master == 0) { - do_unbind_listener(listener, 1); - } + HA_SPIN_UNLOCK(LISTENER_LOCK, &listener->lock); }