BUG/MEDIUM: mworker: don't free the wrong child when not found

A bug occurs when the sigchld handler is called and a child which is
not in the process list just left, or with an empty process list.

The child variable won't be set and left as an uninitialized variable or
set to the wrong child entry, which can lead to a free of this
uninitialized variable or of the wrong child.

This can lead to a crash of the master during a stop or a reload.

It is not supposed to happen with a worker which was created by the
master. A cause could be a fork made by a dependency. (openssl, lua ?)

This patch strengthens the case of the missing child by doing the free
only if the child was found.

This patch must be backported to 1.9.
This commit is contained in:
William Lallemand 2019-03-27 14:19:10 +01:00 committed by Willy Tarreau
parent 5220ef25e3
commit f94afebb94

View File

@ -835,9 +835,12 @@ static void mworker_catch_sigchld(struct sig_handler *sh)
int exitpid = -1; int exitpid = -1;
int status = 0; int status = 0;
struct mworker_proc *child, *it; struct mworker_proc *child, *it;
int childfound;
restart_wait: restart_wait:
childfound = 0;
exitpid = waitpid(-1, &status, WNOHANG); exitpid = waitpid(-1, &status, WNOHANG);
if (exitpid > 0) { if (exitpid > 0) {
if (WIFEXITED(status)) if (WIFEXITED(status))
@ -855,10 +858,11 @@ restart_wait:
LIST_DEL(&child->list); LIST_DEL(&child->list);
close(child->ipc_fd[0]); close(child->ipc_fd[0]);
childfound = 1;
break; break;
} }
if (!children) { if (!children || !childfound) {
ha_warning("Worker %d exited with code %d (%s)\n", exitpid, status, (status >= 128) ? strsignal(status - 128) : "Exit"); ha_warning("Worker %d exited with code %d (%s)\n", exitpid, status, (status >= 128) ? strsignal(status - 128) : "Exit");
} else { } else {
/* check if exited child was in the current children list */ /* check if exited child was in the current children list */
@ -875,8 +879,8 @@ restart_wait:
ha_warning("Former worker #%d (%d) exited with code %d (%s)\n", child->relative_pid, exitpid, status, (status >= 128) ? strsignal(status - 128) : "Exit"); ha_warning("Former worker #%d (%d) exited with code %d (%s)\n", child->relative_pid, exitpid, status, (status >= 128) ? strsignal(status - 128) : "Exit");
delete_oldpid(exitpid); delete_oldpid(exitpid);
} }
free(child);
} }
free(child);
/* do it again to check if it was the last worker */ /* do it again to check if it was the last worker */
goto restart_wait; goto restart_wait;