Do not use O_CLOEXEC for open() in the broker process.

sandbox/linux/services/broker_process.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sandbox/linux/services/broker_process.h"
 
 #include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/syscall.h>
@@ skipping 25 matching lines @@
 // O_CLOEXEC is tricky because in theory another thread could call execve()
 // before special treatment is made on the client, so a client needs to call
 // recvmsg(2) with MSG_CMSG_CLOEXEC.
 // To make things worse, there are two CLOEXEC related flags, FD_CLOEXEC (see
 // F_GETFD in fcntl(2)) and O_CLOEXEC (see F_GETFL in fcntl(2)). O_CLOEXEC
 // doesn't affect the semantics on execve(), it's merely a note that the
 // descriptor was originally opened with O_CLOEXEC as a flag. And it is sent
 // over unix sockets just fine, so a receiver that would (incorrectly) look at
 // O_CLOEXEC instead of FD_CLOEXEC may be tricked in thinking that the file
 // descriptor will or won't be closed on execve().
-// Since we have to account for buggy userland (see crbug.com/237283), we will
-// open(2) the file with O_CLOEXEC in the broker process if necessary, in
-// addition to calling recvmsg(2) with MSG_CMSG_CLOEXEC.
 static const int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
 
 // Check whether |requested_filename| is in |allowed_file_names|.
 // See GetFileNameIfAllowedToOpen() for an explanation of |file_to_open|.
 // async signal safe if |file_to_open| is NULL.
 // TODO(jln): assert signal safety.
 bool GetFileNameInWhitelist(const std::vector<std::string>& allowed_file_names,
                             const char* requested_filename,
                             const char** file_to_open) {
   if (file_to_open && *file_to_open) {
@@ skipping 15 matching lines @@
       return true;
     }
   }
   return false;
 }
 
 // We maintain a list of flags that have been reviewed for "sanity" and that
 // we're ok to allow in the broker.
 // I.e. here is where we wouldn't add O_RESET_FILE_SYSTEM.
 bool IsAllowedOpenFlags(int flags) {
-  // First, check the access mode
+  // First, check the access mode.
   const int access_mode = flags & O_ACCMODE;
   if (access_mode != O_RDONLY && access_mode != O_WRONLY &&
       access_mode != O_RDWR) {
     return false;
   }
 
   // We only support a 2-parameters open, so we forbid O_CREAT.
   if (flags & O_CREAT) {
     return false;
   }
 
   // Some flags affect the behavior of the current process. We don't support
   // them and don't allow them for now.
-  if (flags & kCurrentProcessOpenFlagsMask) {
-    // We make an exception for O_CLOEXEC. Buggy userland could check for
-    // O_CLOEXEC and the only way to set it is to originally open with this
-    // flag. See the comment around kCurrentProcessOpenFlagsMask.
-    if (!(flags & O_CLOEXEC))
-      return false;
-  }
+  if (flags & kCurrentProcessOpenFlagsMask)
+    return false;
 
   // Now check that all the flags are known to us.
   const int creation_and_status_flags = flags & ~O_ACCMODE;
 
   const int known_flags =
     O_APPEND | O_ASYNC | O_CLOEXEC | O_CREAT | O_DIRECT |
     O_DIRECTORY | O_EXCL | O_LARGEFILE | O_NOATIME | O_NOCTTY |
     O_NOFOLLOW | O_NONBLOCK | O_NDELAY | O_SYNC | O_TRUNC;
 
   const int unknown_flags = ~known_flags;
@@ skipping 95 matching lines @@
     return -EFAULT;
 
   // For this "remote system call" to work, we need to handle any flag that
   // cannot be sent over a Unix socket in a special way.
   // See the comments around kCurrentProcessOpenFlagsMask.
   if (syscall_type == kCommandOpen && (flags & kCurrentProcessOpenFlagsMask)) {
     // This implementation only knows about O_CLOEXEC, someone needs to look at
     // this code if other flags are added.
     RAW_CHECK(kCurrentProcessOpenFlagsMask == O_CLOEXEC);
     recvmsg_flags |= MSG_CMSG_CLOEXEC;
+    flags &= ~O_CLOEXEC;
   }
 
   // There is no point in forwarding a request that we know will be denied.
   // Of course, the real security check needs to be on the other side of the
   // IPC.
   if (fast_check_in_client_) {
     if (syscall_type == kCommandOpen &&
         !GetFileNameIfAllowedToOpen(pathname, flags, NULL)) {
       return -EPERM;
     }
@@ skipping 283 matching lines @@
           GetFileNameInWhitelist(allowed_w_files_, requested_filename,
                                  file_to_open);
       return allowed_for_read_and_write;
     }
     default:
       return false;
   }
 }
 
 }  // namespace sandbox.