From owner-freebsd-hackers@FreeBSD.ORG Sun Nov 4 14:37:32 2012 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 35DAC405 for ; Sun, 4 Nov 2012 14:37:32 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5012::107]) by mx1.freebsd.org (Postfix) with ESMTP id B97A28FC08 for ; Sun, 4 Nov 2012 14:37:31 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 12C291203BF for ; Sun, 4 Nov 2012 15:37:28 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id DF89F2848C; Sun, 4 Nov 2012 15:37:27 +0100 (CET) Date: Sun, 4 Nov 2012 15:37:27 +0100 From: Jilles Tjoelker To: freebsd-hackers@FreeBSD.org Subject: [patch] rtld: fix fd leak with parallel dlopen and fork/exec Message-ID: <20121104143727.GB35520@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Nov 2012 14:37:32 -0000 Rtld does not set FD_CLOEXEC on its internal file descriptors; therefore, such a file descriptor may be passed to a process created by another thread running in parallel to dlopen() or fdlopen(). The race is easy to trigger with the below dlopen_exec_race.c that performs the two in parallel repeatedly, for example ./dlopen_exec_race /lib/libedit.so.7 | grep lib No other threads are expected to be running during parsing of the hints and libmap files but the file descriptors need not be passed to child processes so I added O_CLOEXEC there as well. The O_CLOEXEC flag is ignored by older kernels so it will not cause breakage when people try the unsupported action of running new rtld on old kernel. However, the fcntl(F_DUPFD_CLOEXEC) will fail with [EINVAL] on an old kernel so this patch will break fdlopen() with old kernels. I suppose this is OK because fdlopen() is not used in the vital parts of buildworld and the like. Index: libexec/rtld-elf/libmap.c =================================================================== --- libexec/rtld-elf/libmap.c (revision 240561) +++ libexec/rtld-elf/libmap.c (working copy) @@ -121,7 +121,7 @@ } } - fd = open(rpath, O_RDONLY); + fd = open(rpath, O_RDONLY | O_CLOEXEC); if (fd == -1) { dbg("lm_parse_file: open(\"%s\") failed, %s", rpath, rtld_strerror(errno)); Index: libexec/rtld-elf/rtld.c =================================================================== --- libexec/rtld-elf/rtld.c (revision 240561) +++ libexec/rtld-elf/rtld.c (working copy) @@ -1598,7 +1598,7 @@ /* Keep from trying again in case the hints file is bad. */ hints = ""; - if ((fd = open(ld_elf_hints_path, O_RDONLY)) == -1) + if ((fd = open(ld_elf_hints_path, O_RDONLY | O_CLOEXEC)) == -1) return (NULL); if (read(fd, &hdr, sizeof hdr) != sizeof hdr || hdr.magic != ELFHINTS_MAGIC || @@ -2046,13 +2046,13 @@ */ fd = -1; if (fd_u == -1) { - if ((fd = open(path, O_RDONLY)) == -1) { + if ((fd = open(path, O_RDONLY | O_CLOEXEC)) == -1) { _rtld_error("Cannot open \"%s\"", path); free(path); return (NULL); } } else { - fd = dup(fd_u); + fd = fcntl(fd_u, F_DUPFD_CLOEXEC, 0); if (fd == -1) { _rtld_error("Cannot dup fd"); free(path); dlopen_exec_race.c: #include #include #include #include #include #include #include #include extern char **environ; static void * dlopen_thread(void *arg) { const char *path = arg; void *handle; for (;;) { handle = dlopen(path, RTLD_LOCAL | RTLD_NOW); if (handle == NULL) continue; dlclose(handle); } return NULL; } static void filestat_loop(void) { int error, status; pid_t pid, wpid; for (;;) { error = posix_spawnp(&pid, "sh", NULL, NULL, (char *[]){ "sh", "-c", "procstat -f $$", NULL }, environ); if (error != 0) errc(1, error, "posix_spawnp"); do wpid = waitpid(pid, &status, 0); while (wpid == -1 && errno == EINTR); if (wpid == -1) err(1, "waitpid"); if (status != 0) errx(1, "sh -c failed"); } } int main(int argc, char *argv[]) { pthread_t td; int error; if (argc != 2) { fprintf(stderr, "Usage: %s dso\n", argv[0]); return 1; } error = pthread_create(&td, NULL, dlopen_thread, argv[1]); if (error != 0) errc(1, error, "pthread_create"); filestat_loop(); return 0; } fdlopen_exec_race.c: #include #include #include #include #include #include #include #include #include #include extern char **environ; static void * dlopen_thread(void *arg) { const char *path = arg; int fd; void *handle; for (;;) { fd = open(path, O_RDONLY | O_CLOEXEC); if (fd == -1) err(1, "open %s", path); handle = fdlopen(fd, RTLD_LOCAL | RTLD_NOW); close(fd); if (handle == NULL) continue; dlclose(handle); } return NULL; } static void filestat_loop(void) { int error, status; pid_t pid, wpid; for (;;) { error = posix_spawnp(&pid, "sh", NULL, NULL, (char *[]){ "sh", "-c", "procstat -f $$", NULL }, environ); if (error != 0) errc(1, error, "posix_spawnp"); do wpid = waitpid(pid, &status, 0); while (wpid == -1 && errno == EINTR); if (wpid == -1) err(1, "waitpid"); if (status != 0) errx(1, "sh -c failed"); } } int main(int argc, char *argv[]) { pthread_t td; int error; if (argc != 2) { fprintf(stderr, "Usage: %s dso\n", argv[0]); return 1; } error = pthread_create(&td, NULL, dlopen_thread, argv[1]); if (error != 0) errc(1, error, "pthread_create"); filestat_loop(); return 0; } -- Jilles Tjoelker