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.

diff --git a/src/haproxy.c b/src/haproxy.c
index cbcc80cdfe2f746dd9340446902542bfb62389c5..30abbd73deaef1a57bad04939f869fd1ffa8eff5 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -835,9 +835,12 @@ static void mworker_catch_sigchld(struct sig_handler *sh)
        int exitpid = -1;
        int status = 0;
        struct mworker_proc *child, *it;
+       int childfound;
 
 restart_wait:
 
+       childfound = 0;
+
        exitpid = waitpid(-1, &status, WNOHANG);
        if (exitpid > 0) {
                if (WIFEXITED(status))
@@ -855,10 +858,11 @@ restart_wait:
 
                        LIST_DEL(&child->list);
                        close(child->ipc_fd[0]);
+                       childfound = 1;
                        break;
                }
 
-               if (!children) {
+               if (!children || !childfound) {
                        ha_warning("Worker %d exited with code %d (%s)\n", exitpid, status, (status >= 128) ? strsignal(status - 128) : "Exit");
                } else {
                        /* 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");
                                delete_oldpid(exitpid);
                        }
+                       free(child);
                }
-               free(child);
 
                /* do it again to check if it was the last worker */
                goto restart_wait;