kern/170369: More O_CLOEXEC flags to plug files being leaked to
child processes
Jukka A. Ukkonen
jau at iki.fi
Sat Aug 4 09:20:06 UTC 2012
>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 at 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:
More information about the freebsd-bugs
mailing list