CLOEXEC support in fdopen, freopen, and mkostemp/mkostemps.

Change-Id: I74ea88e0d4973d6ab3c57da7d8bb643c31592b14

libc/bionic/lfs64_support.cpp

@@ -17,11 +17,20 @@
 #include <ftw.h>
 #include <stdlib.h>
 
-int mkstemp64(char* filename) {
+// Delegation will work in these cases because all the transitive dependencies
-  // Delegation will work in this case because all the transitive dependencies
+// are already 64-bit ready. In particular, we don't have non-O_LARGEFILE
-  // are already 64-bit ready. In particular, we don't have non-O_LARGEFILE
+// open (our open is actually open64) and stat and stat64 are the same.
-  // open (our open is actually open64) and stat and stat64 are the same.
+int mkstemp64(char* path) {
-  return mkstemp(filename);
+  return mkstemp(path);
+}
+int mkostemp64(char* path, int flags) {
+  return mkostemp(path, flags);
+}
+int mkstemps64(char* path, int suffix_length) {
+  return mkstemps(path, suffix_length);
+}
+int mkostemps64(char* path, int suffix_length, int flags) {
+  return mkostemps(path, suffix_length, flags);
 }
 
 typedef int (*ftw_fn)(const char*, const struct stat*, int);

libc/include/fcntl.h

@@ -46,6 +46,7 @@ __BEGIN_DECLS
 #endif
 
 #define O_ASYNC FASYNC
+#define O_RSYNC O_SYNC
 
 #define SPLICE_F_MOVE 1
 #define SPLICE_F_NONBLOCK 2

libc/include/stdlib.h

@@ -59,8 +59,15 @@ extern int clearenv(void);
 
 extern char* mkdtemp(char*);
 extern char* mktemp(char*) __warnattr("mktemp possibly used unsafely; consider using mkstemp");
-extern int mkstemp(char*);
+
+extern int mkostemp64(char*, int);
+extern int mkostemp(char*, int);
+extern int mkostemps64(char*, int, int);
+extern int mkostemps(char*, int, int);
 extern int mkstemp64(char*);
+extern int mkstemp(char*);
+extern int mkstemps64(char*, int);
+extern int mkstemps(char*, int);
 
 extern long strtol(const char *, char **, int);
 extern long long strtoll(const char *, char **, int);

libc/include/unistd.h

@@ -169,9 +169,7 @@ extern ssize_t pwrite64(int, const void *, size_t, off64_t);
 
 extern int dup(int);
 extern int dup2(int, int);
-#if defined(__USE_GNU)
 extern int dup3(int, int, int);
-#endif
 extern int fcntl(int, int, ...);
 extern int ioctl(int, int, ...);
 extern int flock(int, int);

libc/upstream-openbsd/lib/libc/stdio/fdopen.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: fdopen.c,v 1.6 2008/04/21 12:28:35 otto Exp $ */
+/*	$OpenBSD: fdopen.c,v 1.7 2014/08/31 02:21:18 guenther Exp $ */
 /*-
  * Copyright (c) 1990, 1993
  *	The Regents of the University of California.  All rights reserved.
@@ -66,6 +66,7 @@ fdopen(int fd, const char *mode)
 	if ((fp = __sfp()) == NULL)
 		return (NULL);
 	fp->_flags = flags;
+
 	/*
 	 * If opened for appending, but underlying descriptor does not have
 	 * O_APPEND bit set, assert __SAPP so that __swrite() will lseek to
@@ -73,6 +74,13 @@ fdopen(int fd, const char *mode)
 	 */
 	if ((oflags & O_APPEND) && !(fdflags & O_APPEND))
 		fp->_flags |= __SAPP;
+
+	/*
+	 * If close-on-exec was requested, then turn it on if not already
+	 */
+	if ((oflags & O_CLOEXEC) && !((tmp = fcntl(fd, F_GETFD)) & FD_CLOEXEC))
+		fcntl(fd, F_SETFD, tmp | FD_CLOEXEC);
+
 	fp->_file = fd;
 	fp->_cookie = fp;
 	fp->_read = __sread;

libc/upstream-openbsd/lib/libc/stdio/freopen.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: freopen.c,v 1.13 2009/11/09 00:18:27 kurt Exp $ */
+/*	$OpenBSD: freopen.c,v 1.14 2014/08/31 02:21:18 guenther Exp $ */
 /*-
  * Copyright (c) 1990, 1993
  *	The Regents of the University of California.  All rights reserved.
@@ -134,7 +134,7 @@ freopen(const char *file, const char *mode, FILE *fp)
 	 * assume stderr is always fd STDERR_FILENO, even if being freopen'd.
 	 */
 	if (wantfd >= 0 && f != wantfd) {
-		if (dup2(f, wantfd) >= 0) {
+		if (dup3(f, wantfd, oflags & O_CLOEXEC) >= 0) {
 			(void) close(f);
 			f = wantfd;
 		}

libc/upstream-openbsd/lib/libc/stdio/mktemp.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: mktemp.c,v 1.33 2014/05/06 22:55:27 millert Exp $ */
+/*	$OpenBSD: mktemp.c,v 1.34 2014/08/31 02:21:18 guenther Exp $ */
 /*
  * Copyright (c) 1996-1998, 2008 Theo de Raadt
  * Copyright (c) 1997, 2008-2009 Todd C. Miller
@@ -35,12 +35,14 @@
 #define NUM_CHARS	(sizeof(TEMPCHARS) - 1)
 #define MIN_X		6
 
+#define MKOTEMP_FLAGS	(O_APPEND | O_CLOEXEC | O_DSYNC | O_RSYNC | O_SYNC)
+
 #ifndef nitems
 #define nitems(_a)	(sizeof((_a)) / sizeof((_a)[0]))
 #endif
 
 static int
-mktemp_internal(char *path, int slen, int mode)
+mktemp_internal(char *path, int slen, int mode, int flags)
 {
 	char *start, *cp, *ep;
 	const char *tempchars = TEMPCHARS;
@@ -63,6 +65,12 @@ mktemp_internal(char *path, int slen, int mode)
 		return(-1);
 	}
 
+	if (flags & ~MKOTEMP_FLAGS) {
+		errno = EINVAL;
+		return(-1);
+	}
+	flags |= O_CREAT | O_EXCL | O_RDWR;
+
 	tries = INT_MAX;
 	do {
 		cp = start;
@@ -85,7 +93,7 @@ mktemp_internal(char *path, int slen, int mode)
 				return(errno == ENOENT ? 0 : -1);
 			break;
 		case MKTEMP_FILE:
-			fd = open(path, O_CREAT|O_EXCL|O_RDWR, S_IRUSR|S_IWUSR);
+			fd = open(path, flags, S_IRUSR|S_IWUSR);
 			if (fd != -1 || errno != EEXIST)
 				return(fd);
 			break;
@@ -107,7 +115,7 @@ char *_mktemp(char *);
 char *
 _mktemp(char *path)
 {
-	if (mktemp_internal(path, 0, MKTEMP_NAME) == -1)
+	if (mktemp_internal(path, 0, MKTEMP_NAME, 0) == -1)
 		return(NULL);
 	return(path);
 }
@@ -121,16 +129,28 @@ mktemp(char *path)
 	return(_mktemp(path));
 }
 
+int
+mkostemps(char *path, int slen, int flags)
+{
+	return(mktemp_internal(path, slen, MKTEMP_FILE, flags));
+}
+
 int
 mkstemp(char *path)
 {
-	return(mktemp_internal(path, 0, MKTEMP_FILE));
+	return(mktemp_internal(path, 0, MKTEMP_FILE, 0));
+}
+
+int
+mkostemp(char *path, int flags)
+{
+	return(mktemp_internal(path, 0, MKTEMP_FILE, flags));
 }
 
 int
 mkstemps(char *path, int slen)
 {
-	return(mktemp_internal(path, slen, MKTEMP_FILE));
+	return(mktemp_internal(path, slen, MKTEMP_FILE, 0));
 }
 
 char *
@@ -138,6 +158,6 @@ mkdtemp(char *path)
 {
 	int error;
 
-	error = mktemp_internal(path, 0, MKTEMP_DIR);
+	error = mktemp_internal(path, 0, MKTEMP_DIR, 0);
 	return(error ? NULL : path);
 }

tests/TemporaryFile.h

@@ -17,24 +17,26 @@
 #include <fcntl.h>
 #include <unistd.h>
 
-template<int (*mk_fn)(char*)>
+#include "private/bionic_macros.h"
+
+template <typename T = int (*)(char*)>
 class GenericTemporaryFile {
  public:
-  GenericTemporaryFile(const char* dirpath = NULL) {
+  GenericTemporaryFile(T mk_fn = mkstemp) : mk_fn(mk_fn) {
-    if (dirpath != NULL) {
+    // Since we might be running on the host or the target, and if we're
-      init(dirpath);
+    // running on the host we might be running under bionic or glibc,
-    } else {
+    // let's just try both possible temporary directories and take the
-      // Since we might be running on the host or the target, and if we're
+    // first one that works.
-      // running on the host we might be running under bionic or glibc,
+    init("/data/local/tmp");
-      // let's just try both possible temporary directories and take the
+    if (fd == -1) {
-      // first one that works.
+      init("/tmp");
-      init("/data/local/tmp");
-      if (fd == -1) {
-        init("/tmp");
-      }
     }
   }
 
+  GenericTemporaryFile(const char* dirpath, T mk_fn = mkstemp) : mk_fn(mk_fn) {
+    init(dirpath);
+  }
+
   ~GenericTemporaryFile() {
     close(fd);
     unlink(filename);
@@ -49,13 +51,17 @@ class GenericTemporaryFile {
   char filename[1024];
 
  private:
+  T mk_fn;
+
   void init(const char* tmp_dir) {
     snprintf(filename, sizeof(filename), "%s/TemporaryFile-XXXXXX", tmp_dir);
     fd = mk_fn(filename);
   }
+
+  DISALLOW_COPY_AND_ASSIGN(GenericTemporaryFile);
 };
 
-typedef GenericTemporaryFile<mkstemp> TemporaryFile;
+typedef GenericTemporaryFile<> TemporaryFile;
 
 class TemporaryDir {
  public:
@@ -77,4 +83,5 @@ class TemporaryDir {
     return (mkdtemp(dirname) != NULL);
   }
 
+  DISALLOW_COPY_AND_ASSIGN(TemporaryDir);
 };

tests/ftw_test.cpp

@@ -68,7 +68,7 @@ TEST(ftw, ftw) {
 
 TEST(ftw, ftw64) {
   TemporaryDir td;
-  GenericTemporaryFile<mkstemp64> tf(td.dirname);
+  TemporaryFile tf(td.dirname, mkstemp64);
   ftw64(td.dirname, check_ftw64, 1);
 }
 
@@ -80,6 +80,6 @@ TEST(ftw, nftw) {
 
 TEST(ftw, nftw64) {
   TemporaryDir td;
-  GenericTemporaryFile<mkstemp64> tf(td.dirname);
+  TemporaryFile tf(td.dirname, mkstemp64);
   nftw64(td.dirname, check_nftw64, 1, 0);
 }

tests/stdio_test.cpp

@@ -756,3 +756,43 @@ TEST(stdio, open_memstream_EINVAL) {
   GTEST_LOG_(INFO) << "This test does nothing.\n";
 #endif
 }
+
+TEST(stdio, fdopen_CLOEXEC) {
+  int fd = open("/proc/version", O_RDONLY);
+  ASSERT_TRUE(fd != -1);
+
+  // This fd doesn't have O_CLOEXEC...
+  int flags = fcntl(fd, F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(0, flags & FD_CLOEXEC);
+
+  FILE* fp = fdopen(fd, "re");
+  ASSERT_TRUE(fp != NULL);
+
+  // ...but the new one does.
+  flags = fcntl(fileno(fp), F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(FD_CLOEXEC, flags & FD_CLOEXEC);
+
+  fclose(fp);
+  close(fd);
+}
+
+TEST(stdio, freopen_CLOEXEC) {
+  FILE* fp = fopen("/proc/version", "r");
+  ASSERT_TRUE(fp != NULL);
+
+  // This FILE* doesn't have O_CLOEXEC...
+  int flags = fcntl(fileno(fp), F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(0, flags & FD_CLOEXEC);
+
+  fp = freopen("/proc/version", "re", fp);
+
+  // ...but the new one does.
+  flags = fcntl(fileno(fp), F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(FD_CLOEXEC, flags & FD_CLOEXEC);
+
+  fclose(fp);
+}

tests/stdlib_test.cpp

@@ -162,19 +162,33 @@ TEST(stdlib_DeathTest, getenv_after_main_thread_exits) {
   ASSERT_EXIT(TestBug57421_main(), ::testing::ExitedWithCode(0), "");
 }
 
+TEST(stdlib, mkostemp64) {
+  TemporaryFile tf([](char* path) { return mkostemp64(path, O_CLOEXEC); });
+  int flags = fcntl(tf.fd, F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(FD_CLOEXEC, flags & FD_CLOEXEC);
+}
+
+TEST(stdlib, mkostemp) {
+  TemporaryFile tf([](char* path) { return mkostemp(path, O_CLOEXEC); });
+  int flags = fcntl(tf.fd, F_GETFD);
+  ASSERT_TRUE(flags != -1);
+  ASSERT_EQ(FD_CLOEXEC, flags & FD_CLOEXEC);
+}
+
+TEST(stdlib, mkstemp64) {
+  TemporaryFile tf(mkstemp64);
+  struct stat64 sb;
+  ASSERT_EQ(0, fstat64(tf.fd, &sb));
+  ASSERT_EQ(O_LARGEFILE, fcntl(tf.fd, F_GETFL) & O_LARGEFILE);
+}
+
 TEST(stdlib, mkstemp) {
   TemporaryFile tf;
   struct stat sb;
   ASSERT_EQ(0, fstat(tf.fd, &sb));
 }
 
-TEST(stdlib, mkstemp64) {
-  GenericTemporaryFile<mkstemp64> tf;
-  struct stat64 sb;
-  ASSERT_EQ(0, fstat64(tf.fd, &sb));
-  ASSERT_EQ(O_LARGEFILE, fcntl(tf.fd, F_GETFL) & O_LARGEFILE);
-}
-
 TEST(stdlib, system) {
   int status;