Add O_CLOEXEC to open calls.

This prevents file descriptors from leaking to child processes.

When compiled for older (pre-2.6.23) kernels which lack support for
O_CLOEXEC there is no change in behavior.  With newer kernels, child
processes will no longer inherit leveldb's file handles, which
reduces the changes of accidentally corrupting the database.

Fixes https://github.com/google/leveldb/issues/623

CMakeLists.txt

@@ -38,6 +38,7 @@ include(CheckCXXSymbolExists)
 # (-std=c11), but do expose the function in standard C++ mode (-std=c++11).
 check_cxx_symbol_exists(fdatasync "unistd.h" HAVE_FDATASYNC)
 check_cxx_symbol_exists(F_FULLFSYNC "fcntl.h" HAVE_FULLFSYNC)
+check_cxx_symbol_exists(O_CLOEXEC "fcntl.h" HAVE_O_CLOEXEC)
 
 include(CheckCXXSourceCompiles)
 

port/port_config.h.in

@@ -15,6 +15,11 @@
 #cmakedefine01 HAVE_FULLFSYNC
 #endif  // !defined(HAVE_FULLFSYNC)
 
+// Define to 1 if you have a definition for O_CLOEXEC in <fcntl.h>.
+#if !defined(HAVE_O_CLOEXEC)
+#cmakedefine01 HAVE_O_CLOEXEC
+#endif  // !defined(HAVE_O_CLOEXEC)
+
 // Define to 1 if you have Google CRC32C.
 #if !defined(HAVE_CRC32C)
 #cmakedefine01 HAVE_CRC32C

util/env_posix.cc

@@ -48,6 +48,13 @@ constexpr const int kDefaultMmapLimit = (sizeof(void*) >= 8) ? 1000 : 0;
 // Can be set using EnvPosixTestHelper::SetReadOnlyMMapLimit.
 int g_mmap_limit = kDefaultMmapLimit;
 
+// Common flags defined for all posix open operations
+#if defined(HAVE_O_CLOEXEC)
+constexpr const int O_FLAGS = O_CLOEXEC;
+#else
+constexpr const int O_FLAGS = 0;
+#endif  // defined(HAVE_O_CLOEXEC)
+
 constexpr const size_t kWritableFileBufferSize = 65536;
 
 Status PosixError(const std::string& context, int error_number) {
@@ -168,7 +175,7 @@ class PosixRandomAccessFile final : public RandomAccessFile {
               char* scratch) const override {
     int fd = fd_;
     if (!has_permanent_fd_) {
-      fd = ::open(filename_.c_str(), O_RDONLY);
+      fd = ::open(filename_.c_str(), O_RDONLY | O_FLAGS);
       if (fd < 0) {
         return PosixError(filename_, errno);
       }
@@ -343,7 +350,7 @@ class PosixWritableFile final : public WritableFile {
       return status;
     }
 
-    int fd = ::open(dirname_.c_str(), O_RDONLY);
+    int fd = ::open(dirname_.c_str(), O_RDONLY | O_FLAGS);
     if (fd < 0) {
       status = PosixError(dirname_, errno);
     } else {
@@ -491,7 +498,7 @@ class PosixEnv : public Env {
 
   Status NewSequentialFile(const std::string& filename,
                            SequentialFile** result) override {
-    int fd = ::open(filename.c_str(), O_RDONLY);
+    int fd = ::open(filename.c_str(), O_RDONLY | O_FLAGS);
     if (fd < 0) {
       *result = nullptr;
       return PosixError(filename, errno);
@@ -504,7 +511,7 @@ class PosixEnv : public Env {
   Status NewRandomAccessFile(const std::string& filename,
                              RandomAccessFile** result) override {
     *result = nullptr;
-    int fd = ::open(filename.c_str(), O_RDONLY);
+    int fd = ::open(filename.c_str(), O_RDONLY | O_FLAGS);
     if (fd < 0) {
       return PosixError(filename, errno);
     }
@@ -536,7 +543,9 @@ class PosixEnv : public Env {
 
   Status NewWritableFile(const std::string& filename,
                          WritableFile** result) override {
-    int fd = ::open(filename.c_str(), O_TRUNC | O_WRONLY | O_CREAT, 0644);
+    int fd = ::open(filename.c_str(),
+                    O_TRUNC | O_WRONLY | O_CREAT | O_FLAGS,
+                    0644);
     if (fd < 0) {
       *result = nullptr;
       return PosixError(filename, errno);
@@ -548,7 +557,9 @@ class PosixEnv : public Env {
 
   Status NewAppendableFile(const std::string& filename,
                            WritableFile** result) override {
-    int fd = ::open(filename.c_str(), O_APPEND | O_WRONLY | O_CREAT, 0644);
+    int fd = ::open(filename.c_str(),
+                    O_APPEND | O_WRONLY | O_CREAT | O_FLAGS,
+                    0644);
     if (fd < 0) {
       *result = nullptr;
       return PosixError(filename, errno);
@@ -618,7 +629,7 @@ class PosixEnv : public Env {
   Status LockFile(const std::string& filename, FileLock** lock) override {
     *lock = nullptr;
 
-    int fd = ::open(filename.c_str(), O_RDWR | O_CREAT, 0644);
+    int fd = ::open(filename.c_str(), O_RDWR | O_CREAT | O_FLAGS, 0644);
     if (fd < 0) {
       return PosixError(filename, errno);
     }
@@ -674,7 +685,7 @@ class PosixEnv : public Env {
   }
 
   Status NewLogger(const std::string& filename, Logger** result) override {
-    std::FILE* fp = std::fopen(filename.c_str(), "w");
+    std::FILE* fp = std::fopen(filename.c_str(), "we");
     if (fp == nullptr) {
       *result = nullptr;
       return PosixError(filename, errno);

util/env_posix_test.cc

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 
+#include <sys/wait.h>
+#include <unistd.h>
+
 #include "leveldb/env.h"
 
 #include "port/port.h"
@@ -31,7 +34,7 @@ TEST(EnvPosixTest, TestOpenOnRead) {
   ASSERT_OK(env_->GetTestDirectory(&test_dir));
   std::string test_file = test_dir + "/open_on_read.txt";
 
-  FILE* f = fopen(test_file.c_str(), "w");
+  FILE* f = fopen(test_file.c_str(), "we");
   ASSERT_TRUE(f != nullptr);
   const char kFileData[] = "abcdefghijklmnopqrstuvwxyz";
   fputs(kFileData, f);
@@ -56,9 +59,106 @@ TEST(EnvPosixTest, TestOpenOnRead) {
   ASSERT_OK(env_->DeleteFile(test_file));
 }
 
+#if defined(HAVE_O_CLOEXEC)
+
+TEST(EnvPosixTest, TestCloseOnExec) {
+  // Test that file handles are not inherited by child processes.
+
+  // Open file handles with each of the open methods.
+  std::string test_dir;
+  ASSERT_OK(env_->GetTestDirectory(&test_dir));
+  std::vector<std::string> test_files = {
+      test_dir + "/close_on_exec_seq.txt",
+      test_dir + "/close_on_exec_rand.txt",
+      test_dir + "/close_on_exec_write.txt",
+      test_dir + "/close_on_exec_append.txt",
+      test_dir + "/close_on_exec_lock.txt",
+      test_dir + "/close_on_exec_log.txt",
+  };
+  for (const std::string& test_file : test_files) {
+    const char kFileData[] = "0123456789";
+    ASSERT_OK(WriteStringToFile(env_, kFileData, test_file));
+  }
+  leveldb::SequentialFile* seqFile = nullptr;
+  leveldb::RandomAccessFile* randFile = nullptr;
+  leveldb::WritableFile* writeFile = nullptr;
+  leveldb::WritableFile* appendFile = nullptr;
+  leveldb::FileLock* lockFile = nullptr;
+  leveldb::Logger* logFile = nullptr;
+  ASSERT_OK(env_->NewSequentialFile(test_files[0], &seqFile));
+  ASSERT_OK(env_->NewRandomAccessFile(test_files[1], &randFile));
+  ASSERT_OK(env_->NewWritableFile(test_files[2], &writeFile));
+  ASSERT_OK(env_->NewAppendableFile(test_files[3], &appendFile));
+  ASSERT_OK(env_->LockFile(test_files[4], &lockFile));
+  ASSERT_OK(env_->NewLogger(test_files[5], &logFile));
+
+  // Fork a child process and wait for it to complete.
+  int pid = fork();
+  if (pid == 0) {
+    const char* const child[] = {"/proc/self/exe", "-cloexec-child", nullptr};
+    execv(child[0], const_cast<char* const*>(child));
+    printf("Error spawning child process: %s\n", strerror(errno));
+    exit(6);
+  }
+  int status;
+  waitpid(pid, &status, 0);
+  ASSERT_EQ(0, WEXITSTATUS(status));
+
+  // cleanup
+  ASSERT_OK(env_->UnlockFile(lockFile));
+  delete seqFile;
+  delete randFile;
+  delete writeFile;
+  delete appendFile;
+  delete logFile;
+  for (const std::string& test_file : test_files) {
+    ASSERT_OK(env_->DeleteFile(test_file));
+  }
+}
+
+#endif  // defined(HAVE_O_CLOEXEC)
+
+int cloexecChild() {
+  // Checks for open file descriptors in the range 3..FD_SETSIZE.
+  for (int i = 3; i < FD_SETSIZE; i++) {
+    int dup_result = dup2(i, i);
+    if (dup_result != -1) {
+      printf("Unexpected open file %d\n", i);
+      char nbuf[28];
+      snprintf(nbuf, 28, "/proc/self/fd/%d", i);
+      char dbuf[1024];
+      int result = readlink(nbuf, dbuf, 1024);
+      if (0 < result && result < 1024) {
+        dbuf[result] = 0;
+        printf("File descriptor %d is %s\n", i, dbuf);
+        if (strstr(dbuf, "close_on_exec_") == nullptr) {
+          continue;
+        }
+      } else if (result >= 1024) {
+        printf("(file name length is too long)\n");
+      } else {
+        printf("Couldn't get file name: %s\n", strerror(errno));
+      }
+      return 3;
+    } else {
+      int e = errno;
+      if (e != EBADF) {
+        printf("Unexpected result reading file handle %d: %s\n", i,
+               strerror(errno));
+        return 4;
+      }
+    }
+  }
+  return 0;
+}
+
 }  // namespace leveldb
 
 int main(int argc, char** argv) {
+  // Check if this is the child process for TestCloseOnExec
+  if (argc > 1 && strcmp(argv[1], "-cloexec-child") == 0) {
+    return leveldb::cloexecChild();
+  }
   // All tests currently run with the same read-only file limits.
   leveldb::EnvPosixTest::SetFileLimits(leveldb::kReadOnlyFileLimit,
                                        leveldb::kMMapLimit);