From b7b2478733567d2f167199e02072f90930986d33 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 21 Jun 2016 15:32:29 +0200 Subject: [PATCH] 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. --- src/checks.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/checks.c b/src/checks.c index c4ac947b6..60f12264d 100644 --- 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);