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