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.
This commit is contained in:
Willy Tarreau 2016-06-21 15:32:29 +02:00
parent 164dd0b6e4
commit b7b2478733

View File

@ -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);