From owner-freebsd-hackers@FreeBSD.ORG Sun Nov 4 14:49:29 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 8E340EA3 for ; Sun, 4 Nov 2012 14:49:29 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kostikbel-1-pt.tunnel.tserv11.ams1.ipv6.he.net [IPv6:2001:470:1f14:13d6::2]) by mx1.freebsd.org (Postfix) with ESMTP id EAF558FC17 for ; Sun, 4 Nov 2012 14:49:28 +0000 (UTC) Received: from tom.home (localhost [127.0.0.1]) by kib.kiev.ua (8.14.5/8.14.5) with ESMTP id qA4EnO3t036943; Sun, 4 Nov 2012 16:49:24 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by tom.home (8.14.5/8.14.5/Submit) id qA4EnOd3036942; Sun, 4 Nov 2012 16:49:24 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 4 Nov 2012 16:49:24 +0200 From: Konstantin Belousov To: Jilles Tjoelker Subject: Re: [patch] rtld: fix fd leak with parallel dlopen and fork/exec Message-ID: <20121104144924.GN73505@kib.kiev.ua> References: <20121104143727.GB35520@stack.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="p0HNO2YbtFeVXwJ3" Content-Disposition: inline In-Reply-To: <20121104143727.GB35520@stack.nl> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=0.2 required=5.0 tests=ALL_TRUSTED, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: freebsd-hackers@freebsd.org 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:49:29 -0000 --p0HNO2YbtFeVXwJ3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Nov 04, 2012 at 03:37:27PM +0100, Jilles Tjoelker wrote: > 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(). >=20 > 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 >=20 > 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. >=20 > 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. The patch should be fine. We definitely do not support running newer binaries on older kernels, so the worries about the fdlopen(3) are not sustained. Please commit. >=20 > Index: libexec/rtld-elf/libmap.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- libexec/rtld-elf/libmap.c (revision 240561) > +++ libexec/rtld-elf/libmap.c (working copy) > @@ -121,7 +121,7 @@ > } > } > =20 > - fd =3D open(rpath, O_RDONLY); > + fd =3D open(rpath, O_RDONLY | O_CLOEXEC); > if (fd =3D=3D -1) { > dbg("lm_parse_file: open(\"%s\") failed, %s", rpath, > rtld_strerror(errno)); > Index: libexec/rtld-elf/rtld.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 =3D ""; > =20 > - if ((fd =3D open(ld_elf_hints_path, O_RDONLY)) =3D=3D -1) > + if ((fd =3D open(ld_elf_hints_path, O_RDONLY | O_CLOEXEC)) =3D=3D -1) > return (NULL); > if (read(fd, &hdr, sizeof hdr) !=3D sizeof hdr || > hdr.magic !=3D ELFHINTS_MAGIC || > @@ -2046,13 +2046,13 @@ > */ > fd =3D -1; > if (fd_u =3D=3D -1) { > - if ((fd =3D open(path, O_RDONLY)) =3D=3D -1) { > + if ((fd =3D open(path, O_RDONLY | O_CLOEXEC)) =3D=3D -1) { > _rtld_error("Cannot open \"%s\"", path); > free(path); > return (NULL); > } > } else { > - fd =3D dup(fd_u); > + fd =3D fcntl(fd_u, F_DUPFD_CLOEXEC, 0); > if (fd =3D=3D -1) { > _rtld_error("Cannot dup fd"); > free(path); >=20 >=20 > dlopen_exec_race.c: >=20 > #include > #include >=20 > #include > #include > #include > #include > #include > #include >=20 > extern char **environ; >=20 > static void * > dlopen_thread(void *arg) > { > const char *path =3D arg; > void *handle; >=20 > for (;;) { > handle =3D dlopen(path, RTLD_LOCAL | RTLD_NOW); > if (handle =3D=3D NULL) > continue; > dlclose(handle); > } > return NULL; > } >=20 > static void > filestat_loop(void) > { > int error, status; > pid_t pid, wpid; >=20 > for (;;) { > error =3D posix_spawnp(&pid, "sh", NULL, NULL, > (char *[]){ "sh", "-c", "procstat -f $$", NULL }, > environ); > if (error !=3D 0) > errc(1, error, "posix_spawnp"); > do > wpid =3D waitpid(pid, &status, 0); > while (wpid =3D=3D -1 && errno =3D=3D EINTR); > if (wpid =3D=3D -1) > err(1, "waitpid"); > if (status !=3D 0) > errx(1, "sh -c failed"); > } > } >=20 > int > main(int argc, char *argv[]) > { > pthread_t td; > int error; >=20 > if (argc !=3D 2) > { > fprintf(stderr, "Usage: %s dso\n", argv[0]); > return 1; > } >=20 > error =3D pthread_create(&td, NULL, dlopen_thread, argv[1]); > if (error !=3D 0) > errc(1, error, "pthread_create"); >=20 > filestat_loop(); >=20 > return 0; > } >=20 > fdlopen_exec_race.c: >=20 > #include > #include >=20 > #include > #include > #include > #include > #include > #include > #include > #include >=20 > extern char **environ; >=20 > static void * > dlopen_thread(void *arg) > { > const char *path =3D arg; > int fd; > void *handle; >=20 > for (;;) { > fd =3D open(path, O_RDONLY | O_CLOEXEC); > if (fd =3D=3D -1) > err(1, "open %s", path); > handle =3D fdlopen(fd, RTLD_LOCAL | RTLD_NOW); > close(fd); > if (handle =3D=3D NULL) > continue; > dlclose(handle); > } > return NULL; > } >=20 > static void > filestat_loop(void) > { > int error, status; > pid_t pid, wpid; >=20 > for (;;) { > error =3D posix_spawnp(&pid, "sh", NULL, NULL, > (char *[]){ "sh", "-c", "procstat -f $$", NULL }, > environ); > if (error !=3D 0) > errc(1, error, "posix_spawnp"); > do > wpid =3D waitpid(pid, &status, 0); > while (wpid =3D=3D -1 && errno =3D=3D EINTR); > if (wpid =3D=3D -1) > err(1, "waitpid"); > if (status !=3D 0) > errx(1, "sh -c failed"); > } > } >=20 > int > main(int argc, char *argv[]) > { > pthread_t td; > int error; >=20 > if (argc !=3D 2) > { > fprintf(stderr, "Usage: %s dso\n", argv[0]); > return 1; > } >=20 > error =3D pthread_create(&td, NULL, dlopen_thread, argv[1]); > if (error !=3D 0) > errc(1, error, "pthread_create"); >=20 > filestat_loop(); >=20 > return 0; > } >=20 > --=20 > Jilles Tjoelker > _______________________________________________ > freebsd-hackers@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org" --p0HNO2YbtFeVXwJ3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlCWgHMACgkQC3+MBN1Mb4g24QCfVN8037vLPSuN26+SSURSo0jF 7usAoJQMVfGhVmD1qkO/J29bHMVGpBhv =to7n -----END PGP SIGNATURE----- --p0HNO2YbtFeVXwJ3--