Date: Sat, 4 Aug 2012 09:14:50 GMT From: "Jukka A. Ukkonen" <jau@iki.fi> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/170369: More O_CLOEXEC flags to plug files being leaked to child processes Message-ID: <201208040914.q749EoWs012326@red.freebsd.org> Resent-Message-ID: <201208040920.q749K5Xn077013@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 170369 >Category: kern >Synopsis: More O_CLOEXEC flags to plug files being leaked to child processes >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: change-request >Submitter-Id: current-users >Arrival-Date: Sat Aug 04 09:20:05 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Jukka A. Ukkonen >Release: FreeBSD 9.1-PRERELEASE >Organization: ----- >Environment: FreeBSD sleipnir 9.1-PRERELEASE FreeBSD 9.1-PRERELEASE #1: Tue Jul 31 15:39:12 EEST 2012 root@sleipnir:/usr/obj/usr/src/sys/Sleipnir amd64 >Description: This is a merged patch to plug many potential file descriptor leaks from parent to child processes when one thread calls a function which opens a file and another thread executes a child process before the thread with the new file descriptor gets time to set FD_CLOEXEC. All of the changes included simply add O_CLOEXEC to the options to open(). Some of the changes included in this merged patch rely on another patch which extends fopen() to set O_CLOEXEC when necessary http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/169320 Do not try this one without applying the dependency as well. >How-To-Repeat: See full description above. >Fix: See full description above. Patch attached with submission follows: --- lib/libc/gen/arc4random.c.orig 2012-08-04 10:16:08.000000000 +0300 +++ lib/libc/gen/arc4random.c 2012-08-04 10:16:20.000000000 +0300 @@ -114,7 +114,7 @@ u_int8_t rnd[KEYSIZE]; } rdat; - fd = _open(RANDOMDEV, O_RDONLY, 0); + fd = _open(RANDOMDEV, O_RDONLY | O_CLOEXEC, 0); done = 0; if (fd >= 0) { if (_read(fd, &rdat, KEYSIZE) == KEYSIZE) --- lib/libc/gen/fmtmsg.c.orig 2012-08-04 10:19:48.000000000 +0300 +++ lib/libc/gen/fmtmsg.c 2012-08-04 10:19:57.000000000 +0300 @@ -87,7 +87,7 @@ if (output == NULL) return (MM_NOCON); if (*output != '\0') { - if ((fp = fopen("/dev/console", "a")) == NULL) { + if ((fp = fopen("/dev/console", "ae")) == NULL) { free(output); return (MM_NOCON); } --- lib/libc/gen/fts-compat.c.orig 2012-08-04 10:26:58.000000000 +0300 +++ lib/libc/gen/fts-compat.c 2012-08-04 10:31:15.000000000 +0300 @@ -218,7 +218,8 @@ * and ".." are all fairly nasty problems. Note, if we can't get the * descriptor we run anyway, just more slowly. */ - if (!ISSET(FTS_NOCHDIR) && (sp->fts_rfd = _open(".", O_RDONLY, 0)) < 0) + if (!ISSET(FTS_NOCHDIR) && + (sp->fts_rfd = _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) SET(FTS_NOCHDIR); return (sp); @@ -347,7 +348,8 @@ (p->fts_info == FTS_SL || p->fts_info == FTS_SLNONE)) { p->fts_info = fts_stat(sp, p, 1); if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { - if ((p->fts_symfd = _open(".", O_RDONLY, 0)) < 0) { + if ((p->fts_symfd = _open(".", + O_RDONLY | O_CLOEXEC, 0)) < 0) { p->fts_errno = errno; p->fts_info = FTS_ERR; } else @@ -438,7 +440,7 @@ p->fts_info = fts_stat(sp, p, 1); if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { if ((p->fts_symfd = - _open(".", O_RDONLY, 0)) < 0) { + _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) { p->fts_errno = errno; p->fts_info = FTS_ERR; } else @@ -579,7 +581,7 @@ ISSET(FTS_NOCHDIR)) return (sp->fts_child = fts_build(sp, instr)); - if ((fd = _open(".", O_RDONLY, 0)) < 0) + if ((fd = _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) return (NULL); sp->fts_child = fts_build(sp, instr); if (fchdir(fd)) @@ -1178,7 +1180,7 @@ newfd = fd; if (ISSET(FTS_NOCHDIR)) return (0); - if (fd < 0 && (newfd = _open(path, O_RDONLY, 0)) < 0) + if (fd < 0 && (newfd = _open(path, O_RDONLY | O_CLOEXEC, 0)) < 0) return (-1); if (_fstat(newfd, &sb)) { ret = -1; --- lib/libc/gen/fts.c.orig 2012-08-04 10:33:20.000000000 +0300 +++ lib/libc/gen/fts.c 2012-08-04 10:34:38.000000000 +0300 @@ -213,7 +213,8 @@ * and ".." are all fairly nasty problems. Note, if we can't get the * descriptor we run anyway, just more slowly. */ - if (!ISSET(FTS_NOCHDIR) && (sp->fts_rfd = _open(".", O_RDONLY, 0)) < 0) + if (!ISSET(FTS_NOCHDIR) && + (sp->fts_rfd = _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) SET(FTS_NOCHDIR); return (sp); @@ -342,7 +343,8 @@ (p->fts_info == FTS_SL || p->fts_info == FTS_SLNONE)) { p->fts_info = fts_stat(sp, p, 1); if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { - if ((p->fts_symfd = _open(".", O_RDONLY, 0)) < 0) { + if ((p->fts_symfd = _open(".", + O_RDONLY | O_CLOEXEC, 0)) < 0) { p->fts_errno = errno; p->fts_info = FTS_ERR; } else @@ -433,7 +435,7 @@ p->fts_info = fts_stat(sp, p, 1); if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) { if ((p->fts_symfd = - _open(".", O_RDONLY, 0)) < 0) { + _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) { p->fts_errno = errno; p->fts_info = FTS_ERR; } else @@ -574,7 +576,7 @@ ISSET(FTS_NOCHDIR)) return (sp->fts_child = fts_build(sp, instr)); - if ((fd = _open(".", O_RDONLY, 0)) < 0) + if ((fd = _open(".", O_RDONLY | O_CLOEXEC, 0)) < 0) return (NULL); sp->fts_child = fts_build(sp, instr); if (fchdir(fd)) { @@ -1145,7 +1147,7 @@ newfd = fd; if (ISSET(FTS_NOCHDIR)) return (0); - if (fd < 0 && (newfd = _open(path, O_RDONLY, 0)) < 0) + if (fd < 0 && (newfd = _open(path, O_RDONLY | O_CLOEXEC, 0)) < 0) return (-1); if (_fstat(newfd, &sb)) { ret = -1; --- lib/libc/gen/getcap.c.orig 2012-08-04 10:39:15.000000000 +0300 +++ lib/libc/gen/getcap.c 2012-08-04 10:41:20.000000000 +0300 @@ -241,7 +241,9 @@ myfd = 0; } else { (void)snprintf(pbuf, sizeof(pbuf), "%s.db", *db_p); - if ((capdbp = dbopen(pbuf, O_RDONLY, 0, DB_HASH, 0)) + if ((capdbp = dbopen(pbuf, + O_RDONLY | O_CLOEXEC, + 0, DB_HASH, 0)) != NULL) { free(record); retval = cdbget(capdbp, &record, name); @@ -264,7 +266,7 @@ *cap = cbuf; return (retval); } else { - fd = _open(*db_p, O_RDONLY, 0); + fd = _open(*db_p, O_RDONLY | O_CLOEXEC, 0); if (fd < 0) continue; myfd = 1; --- lib/libc/gen/getcwd.c.orig 2012-08-04 10:45:41.000000000 +0300 +++ lib/libc/gen/getcwd.c 2012-08-04 10:47:37.000000000 +0300 @@ -140,7 +140,7 @@ /* Open and stat parent directory. */ fd = _openat(dir != NULL ? dirfd(dir) : AT_FDCWD, - "..", O_RDONLY); + "..", O_RDONLY | O_CLOEXEC); if (fd == -1) goto err; if (dir) --- lib/libc/gen/getgrent.c.orig 2012-08-04 10:48:47.000000000 +0300 +++ lib/libc/gen/getgrent.c 2012-08-04 10:50:03.000000000 +0300 @@ -810,7 +810,7 @@ if (st->fp != NULL) rewind(st->fp); else if (stayopen) - st->fp = fopen(_PATH_GROUP, "r"); + st->fp = fopen(_PATH_GROUP, "re"); break; case ENDGRENT: if (st->fp != NULL) { @@ -861,7 +861,7 @@ if (*errnop != 0) return (NS_UNAVAIL); if (st->fp == NULL && - ((st->fp = fopen(_PATH_GROUP, "r")) == NULL)) { + ((st->fp = fopen(_PATH_GROUP, "re")) == NULL)) { *errnop = errno; return (NS_UNAVAIL); } @@ -1251,7 +1251,7 @@ if (st->fp != NULL) rewind(st->fp); else if (stayopen) - st->fp = fopen(_PATH_GROUP, "r"); + st->fp = fopen(_PATH_GROUP, "re"); set_setent(dtab, mdata); (void)_nsdispatch(NULL, dtab, NSDB_GROUP_COMPAT, "setgrent", compatsrc, 0); @@ -1335,7 +1335,7 @@ if (*errnop != 0) return (NS_UNAVAIL); if (st->fp == NULL && - ((st->fp = fopen(_PATH_GROUP, "r")) == NULL)) { + ((st->fp = fopen(_PATH_GROUP, "re")) == NULL)) { *errnop = errno; rv = NS_UNAVAIL; goto fin; --- lib/libc/gen/getnetgrent.c.orig 2012-08-04 10:51:39.000000000 +0300 +++ lib/libc/gen/getnetgrent.c 2012-08-04 10:51:53.000000000 +0300 @@ -173,7 +173,7 @@ if (((stat(_PATH_NETGROUP, &_yp_statp) < 0) && errno == ENOENT) || _yp_statp.st_size == 0) _use_only_yp = _netgr_yp_enabled = 1; - if ((netf = fopen(_PATH_NETGROUP,"r")) != NULL ||_use_only_yp){ + if ((netf = fopen(_PATH_NETGROUP,"re")) != NULL ||_use_only_yp){ /* * Icky: grab the first character of the netgroup file * and turn on NIS if it's a '+'. rewind the stream @@ -197,7 +197,7 @@ return; } #else - if ((netf = fopen(_PATH_NETGROUP, "r"))) { + if ((netf = fopen(_PATH_NETGROUP, "re"))) { #endif if (parse_netgrp(group)) endnetgrent(); --- lib/libc/gen/nlist.c.orig 2012-08-04 10:57:58.000000000 +0300 +++ lib/libc/gen/nlist.c 2012-08-04 10:58:11.000000000 +0300 @@ -66,7 +66,7 @@ { int fd, n; - fd = _open(name, O_RDONLY, 0); + fd = _open(name, O_RDONLY | O_CLOEXEC, 0); if (fd < 0) return (-1); n = __fdnlist(fd, list); --- lib/libc/gen/pututxline.c.orig 2012-08-04 11:04:47.000000000 +0300 +++ lib/libc/gen/pututxline.c 2012-08-04 11:06:54.000000000 +0300 @@ -47,7 +47,7 @@ struct stat sb; int fd; - fd = _open(file, O_CREAT|O_RDWR|O_EXLOCK, 0644); + fd = _open(file, O_CREAT|O_RDWR|O_EXLOCK|O_CLOEXEC, 0644); if (fd < 0) return (NULL); @@ -219,7 +219,7 @@ struct stat sb; int fd; - fd = _open(_PATH_UTX_LASTLOGIN, O_RDWR, 0644); + fd = _open(_PATH_UTX_LASTLOGIN, O_RDWR | O_CLOEXEC, 0644); if (fd < 0) return; @@ -253,7 +253,7 @@ vec[1].iov_len = l; l = htobe16(l); - fd = _open(_PATH_UTX_LOG, O_CREAT|O_WRONLY|O_APPEND, 0644); + fd = _open(_PATH_UTX_LOG, O_CREAT|O_WRONLY|O_APPEND|O_CLOEXEC, 0644); if (fd < 0) return (-1); if (_writev(fd, vec, 2) == -1) --- lib/libc/gen/readpassphrase.c.orig 2012-08-04 11:09:43.000000000 +0300 +++ lib/libc/gen/readpassphrase.c 2012-08-04 11:11:13.000000000 +0300 @@ -68,7 +68,7 @@ * stdin and write to stderr unless a tty is required. */ if ((flags & RPP_STDIN) || - (input = output = _open(_PATH_TTY, O_RDWR)) == -1) { + (input = output = _open(_PATH_TTY, O_RDWR | O_CLOEXEC)) == -1) { if (flags & RPP_REQUIRE_TTY) { errno = ENOTTY; return(NULL); --- lib/libc/gen/sem_new.c.orig 2012-08-04 11:14:23.000000000 +0300 +++ lib/libc/gen/sem_new.c 2012-08-04 11:14:31.000000000 +0300 @@ -198,7 +198,7 @@ goto error; } - fd = _open(path, flags|O_RDWR, mode); + fd = _open(path, flags|O_RDWR|O_CLOEXEC, mode); if (fd == -1) goto error; if (flock(fd, LOCK_EX) == -1) --- lib/libc/gen/syslog.c.orig 2012-08-04 11:18:24.000000000 +0300 +++ lib/libc/gen/syslog.c 2012-08-04 11:18:44.000000000 +0300 @@ -300,7 +300,8 @@ * Make sure the error reported is the one from the syslogd failure. */ if (LogStat & LOG_CONS && - (fd = _open(_PATH_CONSOLE, O_WRONLY|O_NONBLOCK, 0)) >= 0) { + (fd = _open(_PATH_CONSOLE, + O_WRONLY|O_NONBLOCK|O_CLOEXEC, 0)) >= 0) { struct iovec iov[2]; struct iovec *v = iov; >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201208040914.q749EoWs012326>