BUG/MEDIUM: external-checks: close all FDs right after the fork()

Lukas Erlacher reported an interesting problem : since we don't close
FDs after the fork() upon external checks, any script executed that
writes data on stdout/stderr will possibly send its data to wrong
places, very likely an existing connection.

After some analysis, the problem is even wider. It's not enough to
just close stdin/stdout/stderr, as all sockets are passed to the
sub-process, and as long as they're not closed, they are usable for
whatever mistake can be done. Furthermore with epoll, such FDs will
continue to be reported after a close() as the underlying file is
not closed yet.

CLOEXEC would be an acceptable workaround except that 1) it adds an
extra syscall on the fast path, and 2) we have no control over FDs
created by external libs (eg: openssl using /dev/crypto, libc using
/dev/random, lua using anything else), so in the end we still need
to close them all.

On some BSD systems there's a closefrom() syscall which could be
very useful for this.

Based on an insightful idea from Simon Horman, we don't close 0/1/2
when we're in verbose mode since they're properly connected to
stdin/stdout/stderr and can become quite useful during debugging
sessions to detect some script output errors or execve() failures.

This fix must be backported to 1.6.

diff --git a/src/checks.c b/src/checks.c
index c4ac947b60514b3f0b78e9c17b4524200d926141..60f12264d99333633a510ccab2b68f538dcf50aa 100644 (file)
--- a/src/checks.c
+++ b/src/checks.c
@@ -1836,6 +1836,14 @@ static int connect_proc_chk(struct task *t)
        if (pid == 0) {
                /* Child */
                extern char **environ;
+               int fd;
+
+               /* close all FDs. Keep stdin/stdout/stderr in verbose mode */
+               fd = (global.mode & (MODE_QUIET|MODE_VERBOSE)) == MODE_QUIET ? 0 : 3;
+
+               while (fd < global.rlimit_nofile)
+                       close(fd++);
+
                environ = check->envp;
                extchk_setenv(check, EXTCHK_HAPROXY_SERVER_CURCONN, ultoa_r(s->cur_sess, buf, sizeof(buf)));
                execvp(px->check_command, check->argv);